[PATCH] fill_user_info: fake user info for 'root' if it can not be found

[Tom Gundersen]

On Mon, Feb 25, 2013 at 7:03 PM, Simon McVittie
<simon.mcvittie at collabora.co.uk> wrote:
> This doesn't seem like something that ought to be wired into every
> sufficiently-low-level library and utility: if it needs to be
> configurable, then it needs to be configurable, and if it doesn't,
> shouldn't libc do this for you?

It would be nice to make glibc use sensible defaults if /etc is empty,
but I didn't propose that as it seems hard to verify that this would
not have any adverse effects. The usage of this stuff in libdbus is
very restricted though, so reviewing it should not be so hard.

> Alternatively, have you considered using a NSS module that "knows who
> root is", analogous to nss-myhostname?

I don't think that improves much over shipping stripped down
/etc/{passwd,nsswich}.

> If none of the alternatives are viable, please open a bug on
> bugs.freedesktop.org so this doesn't get lost.

Ok. I'll see if I can find an alternative first.

> In this situation, don't you fail anyway, further down the function,
> when you getgrouplist() and it doesn't work?

No, worst case it will just use info->primary_gid as the only group
(which we set to 0 above).

>> +    //Make sure 'root' always exists, even if nss is not configured correctly
>
> D-Bus is C89: no C++-style comments, please.

Ok. If I resubmit should I also fix the comments ~10 lines above?

>> +    if ((result != 0 || p != &p_str) && (uid == 0 || strcmp(username_c, "root") == 0))
>> +      {
>> +        _dbus_warn ("Failed to find 'root' user, use fallback entry.");
>
> If this is considered to be normal, it shouldn't warn.

Ok. I'll drop that. I wanted to use something like _dbus_info, but
couldn't find it. Do you have a function for low-priority logging?

>> +        p_str.pw_name = "root";
>> +        p_str.pw_passwd = "x";
>
> I'd prefer to populate @info directly, rather than populating a struct
> passwd and then copying it into @info.

Makes sense.

> You could probably benefit from a fill_user_info_for_root() helper
> shared between the HAVE_GETPWNAM_R and !HAVE_GETPWNAM_R code paths.

Makes sense.

Thanks for the comments!

Cheers,

Tom