[libvirt] [PATCH 1/2] util: Don't fail virGetUserIDByName when user not found
Osier Yang
jyang at redhat.com
Thu Dec 6 17:40:20 UTC 2012
On 2012年12月06日 22: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.
Hmm, okay,
>
> 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:
>
>
> 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;
> }
> }
How about make the logic a bit more clear like:
if (!pw) {
if ((rc == 0) || (rc == EINTR) || (rc == EIO) ||
(rc == EMFILE) || (rc == ENFILE) || (rc == ENOMEM)) {
VIR_DEBUG("User record for user '%s' does not exist", name);
ret = 1;
goto cleanup;
} else {
virReportSystemError(rc, _("Failed to get user record for
name '%s'"),
name);
goto cleanup;
}
}
*uid = pw->pw_uid;
ret = 0;
>
>
>> 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.
>> */
>>
This will be nice.
Osier
More information about the libvir-list
mailing list