[libvirt] [PATCH 1/2] util: Don't fail virGetUserIDByName when user not found

[Peter Krempa]

On 12/06/12 10:25, Osier Yang wrote:
> On 2012年12月05日 20:03, Christophe Fergeau wrote:
>> virGetUserIDByName is documented as returning 1 if the username
>> cannot be found. getpwnam_r is documented as returning:
>> « 0 or ENOENT or ESRCH or EBADF or EPERM or ...  The given name
>> or uid was not found. »
>>   and that:
>> « The formulation given above under "RETURN VALUE" is from POSIX.1-2001.
>> It  does  not  call  "not  found"  an error, hence does not specify what
>> value errno might have in this situation.  But that makes it
>> impossible to
>> recognize errors.  One might argue that according to POSIX errno
>> should be
>> left unchanged if an entry is not found.  Experiments on various
>> UNIX-like
>> systems shows that lots of different values occur in this situation: 0,
>> ENOENT, EBADF, ESRCH, EWOULDBLOCK, EPERM and probably others. »
>>
>> virGetUserIDByName returns an error when the return value of getpwnam_r
>> is non-0. However on my RHEL system, getpwnam_r returns ENOENT when the
>> requested user cannot be found, which then causes virGetUserID not
>> to behave as documented (it returns an error instead of falling back
>> to parsing the passed-in value as an uid).
>>
>> This commit makes virGetUserIDByName ignore the various values listed
>> in getpwnam_r manpage and return a 'user not found' result in such
>> cases.
>> ---
>>   src/util/util.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/util/util.c b/src/util/util.c
>> index 2fd0f2c..df1af7e 100644
>> --- a/src/util/util.c
>> +++ b/src/util/util.c
>> @@ -2530,7 +2530,8 @@ virGetUserIDByName(const char *name, uid_t *uid)
>>           }
>>       }
>>
>> -    if (rc != 0) {
>> +    if ((rc != 0)&&  (rc != ENOENT)&&  (rc != ESRCH)
>> +&&  (rc != EBADF)&&  (rc != EPERM)) {
>
> Correct indention should be:
>
>      if ((rc != 0) && (rc != ENOENT) && (rc != ESRCH) &&
>          (rc != EBADF) && (rc != EPERM))

When I was changing the code the first time, I got rid of this as the 
docs for getpwnam_r seemed clear enough to me that this shouldn't be 
needed. But I was wrong and I'd rather not reintroduce this ugly and 
nondeterministic error checking.

>
> Per the POSIX standard, even these errors are not enough,
> should we give up using getpwnam_r/getgrnam_r, and create
> helpers using getpwent/getgrent to do the lookup work
> ourselves?

Hmm, those look fine to me. I didn't do my research properly when 
changing the code the first time.

>
> <...>
>         The getpwent() function returns a pointer to a passwd structure,
> or NULL if there are no more entries or an error occurs.  If an
>         error occurs, errno is set appropriately.  If one wants to check
> errno after the call, it should be set to zero before the call.
> </...>
>
> <...>
> ERRORS
>         EINTR  A signal was caught.
>
>         EIO    I/O error.
>
>         EMFILE The maximum number (OPEN_MAX) of files was open already
> in the calling process.
>
>         ENFILE The maximum number of files was open already in the system.
>
>         ENOMEM Insufficient memory to allocate passwd structure.
>
>         ERANGE Insufficient buffer space supplied.
> </...>
>
> I think it's pretty easy to distiguish NOT FOUND and other error
> then.

That's true. I'll look into this and check if it's feasible for us to do 
in libvirt.

>
> Regards,
> Osier
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list