PR231653: pw doesn't respect -V when writing pw.conf

[yuripv]

Summary

Separate review for PR231653.

Use the path specified using -V when writing pw.conf if one wasn't expicitly specified using -C.

I'm not entirely sure that this is a correct way to go as man page wording isn't clear here:

-V If this switch is specified, the system /etc/pw.conf will not be sourced for default configuration data, but the file pw.conf in the specified directory will be used instead (or none, if it does not exist).

-D Set default values in /etc/pw.conf configuration file, or a different named configuration file if the -C config option is used.

So -D description doesn't say anything about -V, and that's exactly what I'm seeing when truss'ing pw:

# pw -V /newetc adduser -D -w random:
...
open("/newetc/pw.conf",O_RDONLY,0666)            ERR#2 'No such file or directory'
openat(AT_FDCWD,"/etc/pw.conf",O_RDWR|O_EXLOCK|O_CREAT|O_TRUNC,0644) = 4 (0x4)
...
write(4,"#\n# pw.conf - user/group config"...,1367) = 1367 (0x557)
close(4)                                         = 0 (0x0)
...

This behavior looks strange to me, and I would expect to write to pw.conf file in the same path specified using -V from which we did read the values initially (unless -C is specified explicitly, of course), and this change does just that.

I noticed this while adding test cases for PR231649/D17299 assuming -V set by test helpers should be used. If this is really "not a bug", I can easily use -C there.

[bapt]

if you can provide a regression test, that would be nice

[yuripv]

In D17566#374919, @bapt wrote:

if you can provide a regression test, that would be nice

I can do that, yes, thing is, without this change such test case will (or, at least, will try to) modify the /etc/pw.conf -- if that's not a problem, I will add one.

[eugen_grosbein.net]

While the change seems to be semantically right, I don't really like we have more and more places with hard-coded "pw.conf" in the pw(8) sources.

Could you please to add #define _PW_CONF "pw.conf" to pw.h and use "%s/" _PW_CONF (concatenation of C string literals) instead of "%s/pw.conf" ? And make same change to pw_utils.c' get_userconfig() function too, while you are here.

[yuripv]

Add and use _PW_CONF instead of hardcoding "pw.conf".

[yuripv]

In D17566#374923, @eugen_grosbein.net wrote:

While the change seems to be semantically right, I don't really like we have more and more places with hard-coded "pw.conf" in the pw(8) sources.

Could you please to add #define _PW_CONF "pw.conf" to pw.h and use "%s/" _PW_CONF (concatenation of C string literals) instead of "%s/pw.conf" ? And make same change to pw_utils.c' get_userconfig() function too, while you are here.

Sure, done.

[eugen_grosbein.net]

In D17566#374928, @yuripv wrote:

In D17566#374923, @eugen_grosbein.net wrote:

While the change seems to be semantically right, I don't really like we have more and more places with hard-coded "pw.conf" in the pw(8) sources.

Could you please to add #define _PW_CONF "pw.conf" to pw.h and use "%s/" _PW_CONF (concatenation of C string literals) instead of "%s/pw.conf" ? And make same change to pw_utils.c' get_userconfig() function too, while you are here.

Sure, done.

I have not asked to touch _PATH_PW_CONF in the pw.h and I'm not sure it is completely correct to change it like this, with space and without parenthesis. I'd like to change it back as it was to be sure.

[yuripv]

In D17566#374930, @eugen_grosbein.net wrote:

In D17566#374928, @yuripv wrote:

In D17566#374923, @eugen_grosbein.net wrote:

While the change seems to be semantically right, I don't really like we have more and more places with hard-coded "pw.conf" in the pw(8) sources.

Could you please to add #define _PW_CONF "pw.conf" to pw.h and use "%s/" _PW_CONF (concatenation of C string literals) instead of "%s/pw.conf" ? And make same change to pw_utils.c' get_userconfig() function too, while you are here.

Sure, done.

I have not asked to touch _PATH_PW_CONF in the pw.h and I'm not sure it is completely correct to change it like this, with space and without parenthesis. I'd like to change it back as it was to be sure.

OK.