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>
---
src/lxc/lxc_container.c | 4 ++--
src/security/security_dac.c | 7 +------
src/util/vircommand.c | 3 +--
src/util/virfile.c | 8 --------
src/util/virutil.c | 16 ++++++++--------
tests/commandtest.c | 5 ++---
tools/virt-login-shell-helper.c | 3 +--
7 files changed, 15 insertions(+), 31 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 652697890f..7e460544fb 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2059,8 +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)
+ ngroups = virGetGroupList(virCommandGetUID(cmd), virCommandGetGID(cmd),
+ &groups);
goto cleanup;
ret = 0;
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 4e850e219e..669b90125c 100644
--- 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 81e74deee0..07bee9b12e 100644
--- 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 f66ecd29a2..c4d22921ce 100644
--- 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 bd3bbe3f0d..dc5009f11d 100644
--- 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 aa108ce583..08fbe1801a 100644
--- 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 cb94c49720..1627ba85e5 100644
--- 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;
--
2.45.1On Wed, May 22, 2024 at 18:00:17 +0200, Peter Krempa wrote: > 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> > --- > src/lxc/lxc_container.c | 4 ++-- > src/security/security_dac.c | 7 +------ > src/util/vircommand.c | 3 +-- > src/util/virfile.c | 8 -------- > src/util/virutil.c | 16 ++++++++-------- > tests/commandtest.c | 5 ++--- > tools/virt-login-shell-helper.c | 3 +-- > 7 files changed, 15 insertions(+), 31 deletions(-) > > diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c > index 652697890f..7e460544fb 100644 > --- a/src/lxc/lxc_container.c > +++ b/src/lxc/lxc_container.c > @@ -2059,8 +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) > + ngroups = virGetGroupList(virCommandGetUID(cmd), virCommandGetGID(cmd), > + &groups); > goto cleanup; Looks like leftover goto here. > > ret = 0; ... Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
© 2016 - 2024 Red Hat, Inc.