ufs/extattr.h: Fix documentation of ea_name termination
ClosedPublic
Actions

Authored by cem on Jan 17 2017, 4:03 AM.

Details

Reviewers

kib
mckusick

Commits

rS312391: ufs/extattr.h: Fix documentation of ea_name termination

Summary

The ea_name string is not nul-terminated. Correct the documentation.

Because the subsequent field is padded to 8 bytes, and the padding is
zeroed, the ea_name string will appear to be nul-terminated whenever the
length isn't exactly one (mod eight).

This was introduced in r167010 (2007).

No functional change.

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

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 6816
Build 7022: arc lint + arc unit

Event Timeline

cem updated this revision to Diff 24098.Jan 17 2017, 4:03 AM

cem retitled this revision from to ufs/extattr.h: Fix documentation of ea_name termination.

cem updated this object.

cem edited the test plan for this revision. (Show Details)

cem added reviewers: mckusick, kib.

Additionally, mark the length fields as unsigned. This particularly matters
for the single byte ea_namelength field, which can represent extended attribute
names up to 255 bytes long.

kib added inline comments.Jan 17 2017, 11:24 AM

share/man/man5/fs.5
417–419	Use sizeof() - 1 ?
419	There too.
sys/ufs/ufs/extattr.h
81	Add a line before the content comment, mentioning the padding ?
112	Why removing it ? Even if the macro no longer used in the tree, it is still seems to be useful and might be used by third-party code.

cem added inline comments.Jan 17 2017, 5:09 PM

share/man/man5/fs.5
417–419	Sure, although I expect modern compilers will already compute this constant at compile time for strlen of string constants. In practice, I'm not sure these examples are helpful anyway. Maybe they can just be removed? In the restore code I sprintf the ea_name into a nul-terminated string in one place and just use that instead. You need a nul-terminated name for the extattr_set calls anyway.
sys/ufs/ufs/extattr.h
81	Good idea.
112	It relies upon `strlen(ea_name)`, which is invalid in general. (It is only valid if you create such a structure with `strcpy(ea_name, blah)`.) I think use of strlen(ea_name) suggests that ea_name is nul-terminated, and there's no need for confusion about it. And, the macro was never used in the tree.