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

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.