[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] util: extend virGetUserID and virGetGroupID to support names and IDs



On 10/06/12 04:52, Marcelo Cerri wrote:
This patch updates virGetUserID and virGetGroupID to be able to parse a
user or group name in a similar way to coreutils' chown. This means that
a numeric value with a leading plus sign is always parsed as an ID,
otherwise the functions try to parse the input first as a user or group
name and if this fails they try to parse it as an ID.
---
  src/util/util.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++---------
  1 file changed, 74 insertions(+), 13 deletions(-)

diff --git a/src/util/util.c b/src/util/util.c
index 43fdaf1..694ed3d 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2495,9 +2495,11 @@ char *virGetGroupName(gid_t gid)
      return virGetGroupEnt(gid);
  }

-
-int virGetUserID(const char *name,
-                 uid_t *uid)
+/* Search in the password database for a user id that matches the user name
+ * `name`. Returns 0 on success, -1 on a critical failure or 1 if name cannot
+ * be parsed.
+ */
+static int virGetUserIDByName(const char *name, uid_t *uid)
  {
      char *strbuf;
      struct passwd pwbuf;
@@ -2530,11 +2532,10 @@ int virGetUserID(const char *name,
          }
      }
      if (rc != 0 || pw == NULL) {
-        virReportSystemError(rc,
-                             _("Failed to find user record for name '%s'"),
-                             name);
+        VIR_DEBUG("Failed to find user record for user '%s' (error = %d)",
+                  name, rc);

Although this will work most of the times when an error occurs it will be masked as if the user wasn't found. Unfortunately getpwuid_r and friends have very bad error reporting. The only effective way to distinguish errors from non-existent user entries (according to the manpage) is to check set errno before the call and check it afterwards.

          VIR_FREE(strbuf);
-        return -1;
+        return 1;
      }

      *uid = pw->pw_uid;
@@ -2544,9 +2545,41 @@ int virGetUserID(const char *name,
      return 0;
  }

+/* Try to match a user id based on `user`. The default behavior is to parse
+ * `user` first as a user name and then as a user id. However if `user`
+ * contains a leading '+', the rest of the string is always parsed as a uid.
+ *
+ * Returns 0 on success and -1 otherwise.
+ */
+int virGetUserID(const char *user, uid_t *uid)
+{
+    unsigned int uint_uid;

-int virGetGroupID(const char *name,
-                  gid_t *gid)
+    if (*user == '+') {
+        user++;
+    } else {
+        int rc = virGetUserIDByName(user, uid);
+        if (rc <= 0)
+            return rc;
+    }
+
+    if (virStrToLong_ui(user, NULL, 10, &uint_uid) < 0 ||
+        ((uid_t) uint_uid) != uint_uid) {
+        virReportError(VIR_ERR_INVALID_ARG, _("Failed to parse user '%s'"),
+                       user);
+        return -1;
+    }
+
+    *uid = uint_uid;
+
+    return 0;
+}
+

Otherwise looks ok, so ACK. I'll push this after I get a review on the followup patch that fixes the issue with errors being masked while getting the user entry.

Peter


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]