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

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(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list