virGetGroupList: Refactor and fix callers

Use contemporary style for declarations and automatic memory clearing
for a helper string.

Since the function can't fail any more, remove any mention of returning
errno and remove error checks from all callers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 652697890f8ce7ce3c02f437750d48a81504f7f7..21462affd98c98f82db05850cb4cb1f88584dc60 100644 (file)
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2059,9 +2059,8 @@ static int lxcContainerChild(void *data)
     /* TODO is it safe to call it here or should this call be moved in
      * front of the clone() as otherwise there might be a risk for a
      * deadlock */
-    if ((ngroups = virGetGroupList(virCommandGetUID(cmd), virCommandGetGID(cmd),
-                                   &groups)) < 0)
-        goto cleanup;
+    ngroups = virGetGroupList(virCommandGetUID(cmd), virCommandGetGID(cmd),
+                              &groups);
 
     ret = 0;
  cleanup:

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 4e850e219e33766dc0247af99422bb1088739863..669b90125c302b2bc11c9601f4447bfb41a03f04 100644 (file)
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -524,14 +524,9 @@ static int
 virSecurityDACPreFork(virSecurityManager *mgr)
 {
     virSecurityDACData *priv = virSecurityManagerGetPrivateData(mgr);
-    int ngroups;
 
     g_clear_pointer(&priv->groups, g_free);
-    priv->ngroups = 0;
-    if ((ngroups = virGetGroupList(priv->user, priv->group,
-                                   &priv->groups)) < 0)
-        return -1;
-    priv->ngroups = ngroups;
+    priv->ngroups = virGetGroupList(priv->user, priv->group, &priv->groups);
     return 0;
 }
 

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 81e74deee065b89e17c9a88af10f8eee4f1697f0..07bee9b12e5fb4ea7bf8c47672904004b46e52e0 100644 (file)
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -735,8 +735,7 @@ virExec(virCommand *cmd)
         childerr = null;
     }
 
-    if ((ngroups = virGetGroupList(cmd->uid, cmd->gid, &groups)) < 0)
-        goto cleanup;
+    ngroups = virGetGroupList(cmd->uid, cmd->gid, &groups);
 
     pid = virFork();
 

diff --git a/src/util/virfile.c b/src/util/virfile.c
index f66ecd29a2aff6b515b4a36e138e9ce7488eeed9..c4d22921ce382886aff20a16014168c20b072f7e 100644 (file)
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2285,8 +2285,6 @@ virFileAccessibleAs(const char *path, int mode,
         return access(path, mode);
 
     ngroups = virGetGroupList(uid, gid, &groups);
-    if (ngroups < 0)
-        return -1;
 
     pid = virFork();
 
@@ -2408,8 +2406,6 @@ virFileOpenForked(const char *path,
      * NFS servers. */
 
     ngroups = virGetGroupList(uid, gid, &groups);
-    if (ngroups < 0)
-        return -errno;
 
     if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) < 0) {
         ret = -errno;
@@ -2709,8 +2705,6 @@ virFileRemove(const char *path,
         gid = getegid();
 
     ngroups = virGetGroupList(uid, gid, &groups);
-    if (ngroups < 0)
-        return -errno;
 
     pid = virFork();
 
@@ -2883,8 +2877,6 @@ virDirCreate(const char *path,
         gid = getegid();
 
     ngroups = virGetGroupList(uid, gid, &groups);
-    if (ngroups < 0)
-        return -errno;
 
     pid = virFork();
 

diff --git a/src/util/virutil.c b/src/util/virutil.c
index bd3bbe3f0d07c9e397bdcd97d89fd86f173890d5..dc5009f11d63b2421acf0b940f5e0040d7c976ef 100644 (file)
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -880,14 +880,16 @@ VIR_WARNINGS_NO_POINTER_SIGN
  * storing a malloc'd result into @list. If uid is -1 or doesn't exist in the
  * system database querying of the supplementary groups is skipped.
  *
- * Returns the size of the list on success, or -1 on failure with error
- * reported and errno set. May not be called between fork and exec.
+ * Returns the size of the list. Doesn't have an error path.
+ * May not be called between fork and exec.
  * */
 int
-virGetGroupList(uid_t uid, gid_t gid, gid_t **list)
+virGetGroupList(uid_t uid,
+                gid_t gid,
+                gid_t **list)
 {
     int ret = 0;
-    char *user = NULL;
+    g_autofree char *user = NULL;
     gid_t primary;
 
     *list = NULL;
@@ -925,14 +927,12 @@ virGetGroupList(uid_t uid, gid_t gid, gid_t **list)
 
         for (i = 0; i < ret; i++) {
             if ((*list)[i] == gid)
-                goto cleanup;
+                return ret;
         }
         VIR_APPEND_ELEMENT(*list, i, gid);
-        ret = i;
+        return i;
     }
 
- cleanup:
-    VIR_FREE(user);
     return ret;
 }
 

diff --git a/tests/commandtest.c b/tests/commandtest.c
index aa108ce583db494683f4c991d1832447c725bd78..08fbe1801aebc0b44a5cc1492e8f169e76192155 100644 (file)
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -908,9 +908,8 @@ static int test25(const void *unused G_GNUC_UNUSED)
         goto cleanup;
     }
 
-    if ((ngroups = virGetGroupList(virCommandGetUID(cmd), virCommandGetGID(cmd),
-                                   &groups)) < 0)
-        goto cleanup;
+    ngroups = virGetGroupList(virCommandGetUID(cmd), virCommandGetGID(cmd),
+                              &groups);
 
     /* Now, fork and try to exec a nonexistent binary. */
     pid = virFork();

diff --git a/tools/virt-login-shell-helper.c b/tools/virt-login-shell-helper.c
index cb94c4972076e056266833262de1894a019b6b31..1627ba85e526afdf72313d98b247c2b1e24f82f0 100644 (file)
--- a/tools/virt-login-shell-helper.c
+++ b/tools/virt-login-shell-helper.c
@@ -260,8 +260,7 @@ main(int argc, char **argv)
     if (!(conf = virConfReadFile(login_shell_path, 0)))
         goto cleanup;
 
-    if ((ngroups = virGetGroupList(uid, gid, &groups)) < 0)
-        goto cleanup;
+    ngroups = virGetGroupList(uid, gid, &groups);
 
     if (virLoginShellAllowedUser(conf, name, groups, ngroups) < 0)
         goto cleanup;