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

Tuesday, 24 November 2020

On a Monday in 2020, Ryan Gahagan wrote:
...
From: Barrett Schonefeld <bschoney(a)utexas.edu&gt;

- src/util/virutil.c

Signed-off-by: Barrett Schonefeld <bschoney(a)utexas.edu&gt;
---
 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

    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

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