Re: [libvirt] [PATCHv2 1/3] util: grab primary group when doing user lookup

On 07/09/2013 11:19 PM, Laine Stump wrote: Indeed, that looks a bit nicer. The function is static, so I'm free to do what makes the most sense; I'll post a v3 with this approach. No, I agree with your analysis that a clean implementation is worth the effort, rather than just adding hacks on top of hacks. Here's my incremental patch for the changes (also fixes a bug in virGetXDGDirectory where it could dereference NULL); I'll go ahead and post a v3. diff --git i/src/util/virutil.c w/src/util/virutil.c index 85294ed..5d05fae2 100644 --- i/src/util/virutil.c +++ w/src/util/virutil.c @@ -645,32 +645,30 @@ cleanup: } #ifdef HAVE_GETPWUID_R -enum { - VIR_USER_ENT_DIRECTORY, - VIR_USER_ENT_NAME, -}; - -/* Look up either the name or home directory for a given uid; if group - is non-NULL, also look up the primary group for that user. On - failure, error has been reported, errno is set, and NULL is - returned. */ -static char * -virGetUserEnt(uid_t uid, int field, gid_t *group) +/* Look up fields from the user database for the given user. On + * error, set errno, report the error, and return -1. */ +static int +virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir) { char *strbuf; - char *ret; struct passwd pwbuf; struct passwd *pw = NULL; long val = sysconf(_SC_GETPW_R_SIZE_MAX); size_t strbuflen = val; int rc; + int ret = -1; + + if (name) + *name = NULL; + if (dir) + *dir = NULL; /* sysconf is a hint; if it fails, fall back to a reasonable size */ if (val < 0) strbuflen = 1024; if (VIR_ALLOC_N(strbuf, strbuflen) < 0) - return NULL; + return -1; /* * From the manpage (terrifying but true): @@ -681,21 +679,27 @@ virGetUserEnt(uid_t uid, int field, gid_t *group) */ while ((rc = getpwuid_r(uid, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) { if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) - VIR_FREE(strbuf); - return NULL; + goto cleanup; } if (rc != 0 || pw == NULL) { virReportSystemError(rc, _("Failed to find user record for uid '%u'"), (unsigned int) uid); - VIR_FREE(strbuf); - return NULL; + goto cleanup; } + if (name && VIR_STRDUP(*name, pw->pw_name) < 0) + goto cleanup; if (group) *group = pw->pw_gid; - ignore_value(VIR_STRDUP(ret, field == VIR_USER_ENT_DIRECTORY ? - pw->pw_dir : pw->pw_name)); + if (dir && VIR_STRDUP(*dir, pw->pw_dir) < 0) { + if (name) + VIR_FREE(*name); + goto cleanup; + } + + ret = 0; +cleanup: VIR_FREE(strbuf); return ret; } @@ -745,7 +749,9 @@ static char *virGetGroupEnt(gid_t gid) char *virGetUserDirectory(void) { - return virGetUserEnt(geteuid(), VIR_USER_ENT_DIRECTORY, NULL); + char *ret; + virGetUserEnt(geteuid(), NULL, NULL, &ret); + return ret; } static char *virGetXDGDirectory(const char *xdgenvname, const char *xdgdefdir) @@ -757,8 +763,9 @@ static char *virGetXDGDirectory(const char *xdgenvname, const char *xdgdefdir) if (path && path[0]) { ignore_value(virAsprintf(&ret, "%s/libvirt", path)); } else { - home = virGetUserEnt(geteuid(), VIR_USER_ENT_DIRECTORY, NULL); - ignore_value(virAsprintf(&ret, "%s/%s/libvirt", home, xdgdefdir)); + home = virGetUserDirectory(); + if (home) + ignore_value(virAsprintf(&ret, "%s/%s/libvirt", home, xdgdefdir)); } VIR_FREE(home); @@ -791,7 +798,9 @@ char *virGetUserRuntimeDirectory(void) char *virGetUserName(uid_t uid) { - return virGetUserEnt(uid, VIR_USER_ENT_NAME, NULL); + char *ret; + virGetUserEnt(uid, &ret, NULL, NULL); + return ret; } char *virGetGroupName(gid_t gid) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org