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

[Christophe Fergeau]

On Thu, Dec 06, 2012 at 05:01:22PM +0100, Peter Krempa wrote:
> On 12/06/12 15:37, 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 only report an error when errno
> >is set to one of the values in the posix description of getpwnam_r
> >(which are the same as the ones described in the manpage on my system).
> >---
> >  src/util/util.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> >diff --git a/src/util/util.c b/src/util/util.c
> >index 2fd0f2c..b50563b 100644
> >--- a/src/util/util.c
> >+++ b/src/util/util.c
> >@@ -2531,9 +2531,18 @@ virGetUserIDByName(const char *name, uid_t *uid)
> >      }
> >
> >      if (rc != 0) {
> >-        virReportSystemError(rc, _("Failed to get user record for name '%s'"),
> >-                             name);
> >-        goto cleanup;
> >+       /* We explicitly test for the known error values returned by
> >+        * getpwnam_r as the manpage says:
> >+        * ERRORS
> >+        *   0 or ENOENT or ESRCH or EBADF or EPERM or ...
> >+        *         The given name or uid was not found.
> >+        */
> >+       if ((rc == EINTR) || (rc == EIO) || (rc == EMFILE)
> >+           || (rc == ENFILE) || (rc == ENOMEM)) {
> 
> We usualy write the operator at the end of the previous line of a linebreak.
> 
> 
> >+           virReportSystemError(rc, _("Failed to get user record for name '%s'"),
> >+                                name);
> >+           goto cleanup;
> >+       }
> >      }
> >
> >      if (!pw) {
> >
> 
> ACK with that change.

Thanks, pushed both patches after moving || to the end of the first line.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20121206/b607caa6/attachment-0001.sig>