[libvirt PATCH v2 16/20] commandhelper: Use automatic memory management in parseArguments

Tim Wiederhake posted 20 patches 5 years ago
[libvirt PATCH v2 16/20] commandhelper: Use automatic memory management in parseArguments
Posted by Tim Wiederhake 5 years ago
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
---
 tests/commandhelper.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index 9f0b7f25ac..19dfc09151 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -24,7 +24,9 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 
-#define VIR_NO_GLIB_STDIO /* This file intentionally does not link to libvirt/glib */
+/* This file intentionally does not link to libvirt/glib */
+#define VIR_NO_GLIB_STDIO
+#define cleanup(T, F) __attribute__((cleanup(F))) T
 #include "testutils.h"
 
 #ifndef WIN32
@@ -42,17 +44,27 @@ struct Arguments {
     bool close_stdin;
 };
 
+static void cleanupArguments(struct Arguments **ptr)
+{
+    struct Arguments *args = *ptr;
+
+    if (args)
+        free(args->readfds);
+
+    free(args);
+}
+
 static struct Arguments *parseArguments(int argc, char** argv)
 {
-    struct Arguments* args = NULL;
-    int ret = -1;
+    cleanup(struct Arguments *, cleanupArguments) args = NULL;
+    struct Arguments *ret;
     size_t i;
 
     if (!(args = calloc(1, sizeof(*args))))
-        goto cleanup;
+        return NULL;
 
     if (!(args->readfds = calloc(1, sizeof(*args->readfds))))
-        goto cleanup;
+        return NULL;
 
     args->numreadfds = 1;
     args->readfds[0] = STDIN_FILENO;
@@ -65,12 +77,12 @@ static struct Arguments *parseArguments(int argc, char** argv)
                                     (args->numreadfds + 1) *
                                     sizeof(*args->readfds));
             if (!args->readfds)
-                goto cleanup;
+                return NULL;
 
             if (1 != sscanf(argv[i], "%u%c",
                             &args->readfds[args->numreadfds++], &c)) {
                 printf("Could not parse fd %s\n", argv[i]);
-                goto cleanup;
+                return NULL;
             }
         } else if (STREQ(argv[i], "--check-daemonize")) {
             args->daemonize_check = true;
@@ -79,19 +91,9 @@ static struct Arguments *parseArguments(int argc, char** argv)
         }
     }
 
-    ret = 0;
-
- cleanup:
-    if (ret == 0)
-        return args;
-
-    if (args) {
-        if (args->readfds)
-            free(args->readfds);
-        free(args);
-    }
-
-    return NULL;
+    ret = args;
+    args = NULL;
+    return ret;
 }
 
 static void printArguments(FILE *log, int argc, char** argv)
-- 
2.26.2

Re: [libvirt PATCH v2 16/20] commandhelper: Use automatic memory management in parseArguments
Posted by Peter Krempa 5 years ago
On Mon, Feb 01, 2021 at 12:28:00 +0100, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
> ---
>  tests/commandhelper.c | 42 ++++++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/tests/commandhelper.c b/tests/commandhelper.c
> index 9f0b7f25ac..19dfc09151 100644
> --- a/tests/commandhelper.c
> +++ b/tests/commandhelper.c
> @@ -24,7 +24,9 @@
>  #include <fcntl.h>
>  #include <sys/stat.h>
>  
> -#define VIR_NO_GLIB_STDIO /* This file intentionally does not link to libvirt/glib */
> +/* This file intentionally does not link to libvirt/glib */
> +#define VIR_NO_GLIB_STDIO

Spurious line break.

Re: [libvirt PATCH v2 16/20] commandhelper: Use automatic memory management in parseArguments
Posted by Tim Wiederhake 5 years ago
On Tue, 2021-02-02 at 14:59 +0100, Peter Krempa wrote:
> On Mon, Feb 01, 2021 at 12:28:00 +0100, Tim Wiederhake wrote:
> > Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
> > ---
> >  tests/commandhelper.c | 42 ++++++++++++++++++++++-----------------
> > ---
> >  1 file changed, 22 insertions(+), 20 deletions(-)
> > 
> > diff --git a/tests/commandhelper.c b/tests/commandhelper.c
> > index 9f0b7f25ac..19dfc09151 100644
> > --- a/tests/commandhelper.c
> > +++ b/tests/commandhelper.c
> > @@ -24,7 +24,9 @@
> >  #include <fcntl.h>
> >  #include <sys/stat.h>
> >  
> > -#define VIR_NO_GLIB_STDIO /* This file intentionally does not link
> > to libvirt/glib */
> > +/* This file intentionally does not link to libvirt/glib */
> > +#define VIR_NO_GLIB_STDIO
> 
> Spurious line break.
> 

Intentional. The comment now refers to not only the "#define
VIR_NO_GLIB_STDIO" but also serves as a justification for the custom
cleanup macro that otherwise should be replaced by some glib g_*
construct.