setusercontext(): umask: Set it only once (in the common case)

[olce]

Summary

Simplify the code and make it more coherent (umask was the only context
setting not modified by setlogincontext() directly).

Preserve the current behavior of not changing the umask if none is
specified in the login class capabilities database, but without the
superfluous umask() dance. (The only exception to this is that
a special value no user is likely to input in the database now stands
for no specification.)

If some user has a 'umask' override in its '~/.login_conf', the umask
will still be set twice as before (as is the case for all other context
settings overriden in '~/.login_conf').

Log a warning in case of an invalid umask specification.

This change makes it apparent that the value of LOGIN_DEFUMASK doesn't
matter. It will be removed in a subsequent commit.

PR: 271747

Test Plan

Local test.

[kib]

lib/libutil/login_cap.h

36 ↗

(On Diff #122651)

This is the public macro, are you sure that removing it has no consequences for third-party sources?

lib/libutil/login_class.c

407

The indent looks wrong, it should be +4 spaces from the previous line.

[olce]

lib/libutil/login_cap.h

36 ↗

(On Diff #122651)

GitHub code search and Google show no use of this symbol (the only results are from copies of the login_cap.h).

lib/libutil/login_class.c

407

Right. Fixing this.

[olce]

Fix indent. Note in the commit message that there are a priori no out-of-tree
consumer for the removed symbol.

[olce]

Fix more style(9) problems.

[emaste]

lib/libutil/login_cap.h

36 ↗

(On Diff #122651)

You can request an exp-run for this patch to be certain it won't affect anything in the ports tree if necessary, but I suspect GitHub + Google results are sufficient to claim it is not an issue.

lib/libutil/login_class.c

397–400

Hrm, login_getcapnum has a somewhat unfortunate interface.

405

Perhaps adding || val == err_val does not change the generated code and does not generate a warning? That might be more clear than the comment.

[emaste]

lib/libutil/login_class.c

415

Suggest { } around the first block, even though it is only one statement it is wrapped over many lines.

(Even if not wrapped over many lines I think it's preferable to use braces on all blocks of an if-else if any do.)

if (x) {
        one_statement();
} else {
        a_lot();
        of_stuff();
        over_here();
}

[olce]

lib/libutil/login_class.c

397–400

Yes... Ideally, the libutil interfaces should be revised, but this public interface has been there for so long that it also means reviewing and changing all the applications using it. Not sure how big of a job this is. Just wanted to avoid it as much as possible for this series.

405

Adding it doesn't change the produced assembly. That said, I find this test strange since it is redundant, which presses me to add a comment about that... So, since in both cases I'm putting a comment, I think I still prefer to forget about the redundant construct.

[olce]

Add braces around a split statement. Update the commit message.

[olce]

lib/libutil/login_cap.h

36 ↗

(On Diff #122651)

I did. Please see PR 276570.

That said, if the exp-run doesn't pass because of this interface change, I'll just restore the suppressed #define, this is not per-se a blocker situation for this dev.

[olce]

Delay removing LOGIN_DEFUMASK in another commit (not to be MFCed now).