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>

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),
+    ngroups = virGetGroupList(virCommandGetUID(cmd), virCommandGetGID(cmd),
-                                   &groups)) < 0)
+                              &groups);
-        goto cleanup;
 
     ret = 0;
  cleanup:

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;
+    priv->ngroups = virGetGroupList(priv->user, priv->group, &priv->groups);
-    if ((ngroups = virGetGroupList(priv->user, priv->group,
-                                   &priv->groups)) < 0)
-        return -1;
-    priv->ngroups = ngroups;
     return 0;
 }
 

src/util/vircommand.c

@@ -735,8 +735,7 @@ virExec(virCommand *cmd)
         childerr = null;
     }
 
-    if ((ngroups = virGetGroupList(cmd->uid, cmd->gid, &groups)) < 0)
+    ngroups = virGetGroupList(cmd->uid, cmd->gid, &groups);
-        goto cleanup;
 
     pid = virFork();
 

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();
 

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
+ * Returns the size of the list. Doesn't have an error path.
- * reported and errno set. May not be called between fork and exec.
+ * 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;
 }
 

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),
+    ngroups = virGetGroupList(virCommandGetUID(cmd), virCommandGetGID(cmd),
-                                   &groups)) < 0)
+                              &groups);
-        goto cleanup;
 
     /* Now, fork and try to exec a nonexistent binary. */
     pid = virFork();

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)
+    ngroups = virGetGroupList(uid, gid, &groups);
-        goto cleanup;
 
     if (virLoginShellAllowedUser(conf, name, groups, ngroups) < 0)
         goto cleanup;