[libvirt] [PATCH] util: Improve error reporting from getgrnam_r and friends

[Peter Krempa]

Error reporting for getgrnam_r() isn't that broken as in getgrnam().

This patch enhances virGetUserIDByName() and virGetGroupIDByName() so
that they error out if retrieval of the information failed but just log
a debug message if the entry was not found.

>From the man page for getgrnam_r():

RETURN VALUE
  ...
  On success, getgrnam_r() and getgrgid_r() return zero, and set *result
  to grp.  If no matching group record was found, these functions return
  0 and store NULL in *result.  In case of error, an error number is
  returned, and NULL is stored in *result.
---
This patch has to be applied on top of:
http://www.redhat.com/archives/libvir-list/2012-October/msg00190.html


 src/util/util.c | 70 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 36 insertions(+), 34 deletions(-)

diff --git a/src/util/util.c b/src/util/util.c
index 694ed3d..8ddae0d 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2501,12 +2501,13 @@ char *virGetGroupName(gid_t gid)
  */
 static int virGetUserIDByName(const char *name, uid_t *uid)
 {
-    char *strbuf;
+    char *strbuf = NULL;
     struct passwd pwbuf;
     struct passwd *pw = NULL;
     long val = sysconf(_SC_GETPW_R_SIZE_MAX);
     size_t strbuflen = val;
     int rc;
+    int ret = -1;

     /* sysconf is a hint; if it fails, fall back to a reasonable size */
     if (val < 0)
@@ -2514,35 +2515,35 @@ static int virGetUserIDByName(const char *name, uid_t *uid)

     if (VIR_ALLOC_N(strbuf, strbuflen) < 0) {
         virReportOOMError();
-        return -1;
+        goto cleanup;
     }

-    /*
-     * 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.
-     */
     while ((rc = getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) {
         if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) {
             virReportOOMError();
-            VIR_FREE(strbuf);
-            return -1;
+            goto cleanup;
         }
     }
-    if (rc != 0 || pw == NULL) {
-        VIR_DEBUG("Failed to find user record for user '%s' (error = %d)",
-                  name, rc);
-        VIR_FREE(strbuf);
-        return 1;
+
+    if (rc != 0) {
+        virReportSystemError(rc, _("Failed to get user record for name '%s'"),
+                             name);
+        goto cleanup;
+    }
+
+    if (!pw) {
+        VIR_DEBUG("User record for user '%s' does not exist", name);
+        ret = 1;
+        goto cleanup;
     }

     *uid = pw->pw_uid;
+    ret = 0;

+cleanup:
     VIR_FREE(strbuf);

-    return 0;
+    return ret;
 }

 /* Try to match a user id based on `user`. The default behavior is to parse
@@ -2581,12 +2582,13 @@ int virGetUserID(const char *user, uid_t *uid)
  */
 static int virGetGroupIDByName(const char *name, gid_t *gid)
 {
-    char *strbuf;
+    char *strbuf = NULL;
     struct group grbuf;
     struct group *gr = NULL;
     long val = sysconf(_SC_GETGR_R_SIZE_MAX);
     size_t strbuflen = val;
     int rc;
+    int ret = -1;

     /* sysconf is a hint; if it fails, fall back to a reasonable size */
     if (val < 0)
@@ -2594,35 +2596,35 @@ static int virGetGroupIDByName(const char *name, gid_t *gid)

     if (VIR_ALLOC_N(strbuf, strbuflen) < 0) {
         virReportOOMError();
-        return -1;
+        goto cleanup;
     }

-    /*
-     * 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.
-     */
     while ((rc = getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr)) == ERANGE) {
         if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) {
             virReportOOMError();
-            VIR_FREE(strbuf);
-            return -1;
+            goto cleanup;
         }
     }
-    if (rc != 0 || gr == NULL) {
-        VIR_DEBUG("Failed to find group record for group '%s' (error = %d)",
-                  name, rc);
-        VIR_FREE(strbuf);
-        return 1;
+
+    if (rc != 0) {
+        virReportSystemError(rc, _("Failed to get group record for name '%s'"),
+                             name);
+        goto cleanup;
+    }
+
+    if (!gr) {
+        VIR_DEBUG("Group record for group '%s' does not exist", name);
+        ret = 1;
+        goto cleanup;
     }

     *gid = gr->gr_gid;
+    ret = 0;

+cleanup:
     VIR_FREE(strbuf);

-    return 0;
+    return ret;
 }

 /* Try to match a group id based on `group`. The default behavior is to parse
-- 
1.7.12