Page MenuHomeFreeBSD

pw: add new configuration keyword logmode in pw.conf
Needs RevisionPublic

Authored by sbz on Apr 13 2020, 10:21 AM.



The permission of the created logfile were not possible to customize as
they were hardcoded inside pw_log() function.

Add a new field logmode in the userconf struct to expose a new
configuration keyword in pw.conf, it will make possible to tweak the mode
of the logfile created when invoking pw(8).

PR: 216897

Test Plan
[sbz@devbox /usr/src/usr.sbin/pw]$ (cd /usr/tests/usr.sbin/pw/ && kyua test && kyua report)
===> Summary
Results read from /root/.kyua/store/results.usr_tests_usr.sbin_pw.20200413-101148-790104.db
Test cases: 92 total, 0 skipped, 0 expected failures, 0 broken, 0 failed
Total time: 2.902s

Diff Detail

Lint Passed
No Test Coverage
Build Status
Buildable 30454
Build 28211: arc lint + arc unit

Event Timeline

bapt requested changes to this revision.Apr 15 2020, 2:46 PM

We need unit tests, pw(8) is very sensitive and we don't add any features without regression tests


I think we should test for EINVAL and report an error for the users like we do for _UC_MINUID

This revision now requires changes to proceed.Apr 15 2020, 2:46 PM
ngie requested changes to this revision.Apr 18 2020, 12:04 AM

The change looks good functionally. My comments are mostly about the documentation/tests.


This seems like an incorrect place to put this.

This proposed change modifies the entry, like so:

The logfile option allows logging of password file modifications into the nominated logfile.


The logfile keyword is optional. option allows logging of password file modifications into the nominated logfile.

Given that none of the other entries state whether or not directives are required, this change can probably be omitted.


Same comment here as above about "keyword is optional".


I think Default: 0600 should be on a newline, but I'll defer to others who are more keyed in to how mdoc should work.


It would be really nice if this used C99 initializers (I suggest doing this in another commit).


It would probably be a good idea to use a a) non-default value and a b) default value to ensure that a) the logmode value is honored and b) the default value for a logfile is 0600, per the spec.

Note for myself: