Page MenuHomeFreeBSD

mountd(8): parsecred(): Re-order operations for clarity
AcceptedPublic

Authored by olce on Oct 8 2024, 1:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 5, 3:57 AM
Unknown Object (File)
Fri, Nov 1, 2:22 PM
Unknown Object (File)
Tue, Oct 15, 7:01 AM
Unknown Object (File)
Sat, Oct 12, 8:50 AM
Unknown Object (File)
Thu, Oct 10, 5:01 AM
Unknown Object (File)
Thu, Oct 10, 2:02 AM
Unknown Object (File)
Wed, Oct 9, 8:38 AM
Subscribers

Details

Reviewers
rmacklem
Summary

No functional change (intended).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 59846
Build 56731: arc lint + arc unit

Event Timeline

olce requested review of this revision.Oct 8 2024, 1:47 PM

I'll click Accept, although I do not think it makes the code
clearer.
Just make it clear in the commit message that there are
no semantics change/bugfix done by this commit.

This revision is now accepted and ready to land.Sun, Oct 13, 8:45 PM

I'll click Accept,

Thanks.

although I do not think it makes the code clearer.

It fixes the position of the code checking validity of name and setting cr_uid. The current one is both surprising logically (should be close to strtoul()) and wrong with respect to the Explicit credential specified as a colon separated list: comment. That it allows removing the now superfluous r->cr_uid = pw->pw_uid; line in the names == NULL case is a confirmation that it was misplaced. The change also makes it clearer that the part introduced by the previous comment and the one by Credentials specified as those of a user. are alternatives, now that they are not polluted by independent code. Finally, having the fallback part filling separately cr just before exiting, instead of pre-filling the object at start, makes it easy to see that the returned object is consistent without having to check all the function's code to see if it changes only some of the fields.

I perhaps should have specified that, in my view, "code clarity" includes fast reading and being able to quickly prove mentally that some code is correct.

Just make it clear in the commit message that there are
no semantics change/bugfix done by this commit.

I always do that. Will amend the commit message now so as not to forget.

usr.sbin/mountd/mountd.c
3649

There's an unintended change in behavior here, as cr_uid has been set but will be reset to UID_NOBODY after the jump to fallback.

Will be fixed at commit.