Page MenuHomeFreeBSD
Differential D9225

ffs_vnops: Simplify extattr access
ClosedPublic
Actions

Authored by cem on Jan 17 2017, 7:54 PM.
Tags
None
Referenced Files
F81971456: D9225.id24139.diff
Sun, Dec 15, 2:59 AM
F81971255: D9225.diff
Sun, Dec 15, 12:48 AM
Unknown Object (File)
Fri, Dec 13, 3:33 PM
Unknown Object (File)
Wed, Dec 11, 10:05 AM
Unknown Object (File)
Tue, Dec 10, 8:20 PM
Unknown Object (File)
Mar 17 2024, 6:25 AM
Unknown Object (File)
Jan 7 2024, 5:39 PM
Unknown Object (File)
Jan 7 2024, 5:39 PM
View All 53 Files
Subscribers
None

Details

Reviewers
kib
mckusick
Commits
rS312416: ffs_vnops: Simplify extattr access
Summary

As suggested in r167010, use the structure type and macros to access and
modify UFS2 extended attributes.

PR: 216127
Reported by: dewayne at heuristicsystems.com.au
Sponsored by: Dell EMC Isilon

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

cem updated this revision to Diff 24134.Jan 17 2017, 7:54 PM
cem retitled this revision from to ffs_vnops: Simplify extattr access.
cem updated this object.
cem edited the test plan for this revision. (Show Details)
cem added reviewers: kib, mckusick.
kib edited edge metadata.Jan 17 2017, 10:11 PM

This is fine, but I have one suggestion.

In each place which casts char * into the structure, verify the alignment. This might be not strictly needed right now, since from what I see the pointers are malloced, but might help if the code starts mutating in future. I am not sure whether the checks should be asserts or silently return error.

cem added a comment.Jan 17 2017, 11:13 PM
In D9225#190692, @kib wrote:

This is fine, but I have one suggestion.

In each place which casts char * into the structure, verify the alignment. This might be not strictly needed right now, since from what I see the pointers are malloced, but might help if the code starts mutating in future. I am not sure whether the checks should be asserts or silently return error.

Sure, that seems like a good idea. I wasn't too concerned about it because the code takes care to keep struct extattrs (and contents) 8-byte aligned -- and I don't think we require greater alignment outside of e.g. SSE/AVX values.

cem updated this revision to Diff 24139.Jan 18 2017, 12:49 AM
cem edited edge metadata.

Add correct alignment asserts when casting u_char * to struct extattr *.

kib added inline comments.Jan 18 2017, 10:52 AM
sys/ufs/ffs/ffs_vnops.c
1110 ↗(On Diff #24139)

I have some doubts that the expression is the correct C. In particular, the logical ops with the pointers are not defined, AFAIR. Did you compiled the code with INVARIANTS enabled ?

cem updated this revision to Diff 24165.Jan 18 2017, 5:50 PM
cem marked an inline comment as done.

Correct alignment checks by casting through uintptr_t.

kib accepted this revision.Jan 18 2017, 5:58 PM
kib edited edge metadata.
This revision is now accepted and ready to land.Jan 18 2017, 5:58 PM
Closed by commit rS312416: ffs_vnops: Simplify extattr access (authored by cem). · Explain WhyJan 19 2017, 4:46 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
sys/
ufs/
ffs/
190 lines

Diff 24196

View Options

head/sys/ufs/ffs/ffs_vnops.c

Loading...