[libvirt] [PATCH 1/2] util: Don't fail virGetUserIDByName when user not found
Peter Krempa
pkrempa at redhat.com
Thu Dec 6 14:23:16 UTC 2012
On 12/06/12 15:16, Christophe Fergeau wrote:
> On Thu, Dec 06, 2012 at 03:07:32PM +0100, Peter Krempa wrote:
>> On 12/05/12 13: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)) {
>>> virReportSystemError(rc, _("Failed to get user record for name '%s'"),
>>> name);
>>> goto cleanup;
>>>
>>
>> Hm, this is the most elegant solution to the problem I came across.
>> getpwent_r suggested by Osier isn't reentrant as one would think and
>> other options would require too invasive changes into this code.
>
> Another option could be to list the errno values for which we return
> an error rather than listing the errno values for which we don't want to
> return an error:
Hm, maybe that would be better. Those other errors seem to be specified
more deterministic as those nasty tree points at the end of the "not
found" case.
>
>
> if (rc != 0) {
> if ((rc == EINTR) || (rc == EIO) || (rc == EMFILE)
> || (rc == ENFILE) || (rc == ENOMEM)) {
> virReportSystemError(rc, _("Failed to get group record for name %s'"),
> name);
> goto cleanup;
> }
> }
>
>
>> ACK with formatting fixed as Osier suggested and this comment:
>> /*
>> * From the manpage (terrifying but true):
>> *
>> * ERRORS
>> * 0 or ENOENT or ESRCH or EBADF or EPERM or ...
>> * The given name or uid was not found.
>> */
>>
>> placed before the call of the getpwnam_r function. (It was there before.)
>
> I'll also add an errno = 0; before the call as mentioned by Osier. I'll
resetting of errno isn't needed, getpwnam_r doesn't use it
> send a v2 with these changes once you tell me if you prefer the old
> filtering or the one suggested in this email. Thanks,
>
> Christophe
>
Peter
>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
More information about the libvir-list
mailing list