Re: [PATCH 21/30] util: remove cleanup label

[Ján Tomko]

On a Monday in 2020, Ryan Gahagan wrote:

From: Barrett Schonefeld <bscho...@utexas.edu>

- src/util/virutil.c

Signed-off-by: Barrett Schonefeld <bscho...@utexas.edu>
---
src/util/virutil.c | 45 ++++++++++++++++-----------------------------
1 file changed, 16 insertions(+), 29 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index d4b864d5cb..52ab6c6e6d 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -773,7 +773,7 @@ virGetUserIDByName(const char *name, uid_t *uid, bool 
missing_ok)

    while ((rc = getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) {
        if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0)
-            goto cleanup;
+            return ret;

return -1;

That way it's obvious it always returns an error code.
By using ret the reader has to track whether the variable has not been
changed since its initialization.

We've also had bugs in the past with reusal of the 'ret' variable - a
succsesful call left it at '0', while the rest of the code assumed it
was still at -1.

    }

    if (!pw) {
@@ -785,15 +785,13 @@ virGetUserIDByName(const char *name, uid_t *uid, bool 
missing_ok)
                     name, g_strerror(rc));
        }

-        ret = 1;
-        goto cleanup;
+        return 1;
    }

    if (uid)
        *uid = pw->pw_uid;

    ret = 0;

- cleanup:
    return ret;

return 0;

}

@@ -852,7 +850,7 @@ virGetGroupIDByName(const char *name, gid_t *gid, bool 
missing_ok)

    while ((rc = getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr)) == ERANGE) {
        if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0)
-            goto cleanup;
+            return ret;
    }

    if (!gr) {
@@ -864,16 +862,13 @@ virGetGroupIDByName(const char *name, gid_t *gid, bool 
missing_ok)
                     name, g_strerror(rc));
        }

-        ret = 1;
-        goto cleanup;
+        return 1;
    }

    if (gid)
        *gid = gr->gr_gid;
-    ret = 0;

- cleanup:
-    return ret;
+    return 0;
}

/* Try to match a group id based on `group`. The default behavior is to parse
@@ -981,18 +976,17 @@ virGetGroupList(uid_t uid, gid_t gid, gid_t **list)

        for (i = 0; i < ret; i++) {
            if ((*list)[i] == gid)
-                goto cleanup;
+                return ret;
        }
        if (VIR_APPEND_ELEMENT(*list, i, gid) < 0) {

            ret = -1;
            VIR_FREE(*list);
-            goto cleanup;
+            return ret;

return -1;

and the assingment above can be dropped.

        } else {
            ret = i;
        }
    }

- cleanup:
    return ret;
}

@@ -1409,18 +1403,17 @@ virSetDeviceUnprivSGIO(const char *path,
    if (!virFileExists(sysfs_path)) {
        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                       _("unpriv_sgio is not supported by this kernel"));
-        goto cleanup;
+        return ret;

-1

    }

    val = g_strdup_printf("%d", unpriv_sgio);

    if ((rc = virFileWriteStr(sysfs_path, val, 0)) < 0) {
        virReportSystemError(-rc, _("failed to set %s"), sysfs_path);
-        goto cleanup;
+        return ret;

-1

    }

    ret = 0;
- cleanup:
    return ret;

return 0;
and the ret variable can be dropped

}

@@ -1440,11 +1433,11 @@ virGetDeviceUnprivSGIO(const char *path,
    if (!virFileExists(sysfs_path)) {
        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                       _("unpriv_sgio is not supported by this kernel"));
-        goto cleanup;
+        return ret;

-1

    }

    if (virFileReadAll(sysfs_path, 1024, &buf) < 0)
-        goto cleanup;
+        return ret;

-1

    if ((tmp = strchr(buf, '\n')))
        *tmp = '\0';
@@ -1452,12 +1445,10 @@ virGetDeviceUnprivSGIO(const char *path,
    if (virStrToLong_i(buf, NULL, 10, unpriv_sgio) < 0) {
        virReportError(VIR_ERR_INTERNAL_ERROR,
                       _("failed to parse value of %s"), sysfs_path);
-        goto cleanup;
+        return ret;

-1

    }

-    ret = 0;
- cleanup:
-    return ret;
+    return 0;
}

Jano

signature.asc
Description: PGP signature