Use 'g_autofree' to clean both 'path' and 'xml' which mandates initialization and get rid of the 'cleanup' label and 'ret variable. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/virnetworkportdef.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index b58e2ccae0..a4cffea8b6 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -444,29 +444,24 @@ virNetworkPortDefSaveStatus(virNetworkPortDef *def, const char *dir) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - char *path; - char *xml = NULL; - int ret = -1; + g_autofree char *path = NULL; + g_autofree char *xml = NULL; virUUIDFormat(def->uuid, uuidstr); if (virFileMakePath(dir) < 0) - goto cleanup; + return -1; if (!(path = virNetworkPortDefConfigFile(dir, uuidstr))) - goto cleanup; + return -1; if (!(xml = virNetworkPortDefFormat(def))) - goto cleanup; + return -1; if (virXMLSaveFile(path, uuidstr, "net-port-create", xml) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - VIR_FREE(xml); - VIR_FREE(path); - return ret; + return 0; } -- 2.24.1

Use 'g_autoptr' which mandates initialization for 'hostname' and also for 'domain' to allow full refactor of the cleanup path. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tools/virsh-domain.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 9d0f7d68d2..8591e483a5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11885,9 +11885,8 @@ VIR_ENUM_IMPL(virshDomainHostnameSource, static bool cmdDomHostname(vshControl *ctl, const vshCmd *cmd) { - char *hostname; - virDomainPtr dom; - bool ret = false; + g_autofree char *hostname = NULL; + g_autoptr(virshDomain) dom = NULL; const char *sourcestr = NULL; int flags = 0; /* Use default value. Drivers can have its own default. */ @@ -11895,14 +11894,14 @@ cmdDomHostname(vshControl *ctl, const vshCmd *cmd) return false; if (vshCommandOptStringReq(ctl, cmd, "source", &sourcestr) < 0) - goto error; + return false; if (sourcestr) { int source = virshDomainHostnameSourceTypeFromString(sourcestr); if (source < 0) { vshError(ctl, _("Unknown data source '%s'"), sourcestr); - goto error; + return false; } switch ((virshDomainHostnameSource) source) { @@ -11920,16 +11919,11 @@ cmdDomHostname(vshControl *ctl, const vshCmd *cmd) hostname = virDomainGetHostname(dom, flags); if (hostname == NULL) { vshError(ctl, "%s", _("failed to get hostname")); - goto error; + return false; } vshPrint(ctl, "%s\n", hostname); - ret = true; - - error: - VIR_FREE(hostname); - virshDomainFree(dom); - return ret; + return true; } /** -- 2.24.1

Our implementation masks GCC warnings of uninitialized use of the passed argument. After changing this I got a load of following warnings: src/conf/virnetworkportdef.c: In function 'virNetworkPortDefSaveStatus': /usr/include/glib-2.0/glib/gmem.h:136:8: error: 'path' may be used uninitialized in this function [-Werror=maybe-uninitialized] 136 | if (_p) \ | ^ src/conf/virnetworkportdef.c:447:11: note: 'path' was declared here 447 | char *path; | ^~~~ For the curious, g_clear_pointer is still safe for arguments with side-effect. Here's the post-processed output of trying to do a VIR_FREE(*(test2++)): do { typedef char _GStaticAssertCompileTimeAssertion_1[(sizeof *(&(*(test2++))) == sizeof (gpointer)) ? 1 : -1] __attribute__((__unused__)); __typeof__((&(*(test2++)))) _pp = (&(*(test2++))); __typeof__(*(&(*(test2++)))) _ptr = *_pp; *_pp = ((void *)0); if (_ptr) (g_free) (_ptr); } while (0) ; Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/libvirt_private.syms | 1 - src/util/viralloc.c | 21 ++------------------- src/util/viralloc.h | 7 +------ 3 files changed, 3 insertions(+), 26 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8563695c32..d5f6d7ec05 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1584,7 +1584,6 @@ virDeleteElementsN; virDispose; virDisposeString; virExpandN; -virFree; virInsertElementsN; virReallocN; virResizeN; diff --git a/src/util/viralloc.c b/src/util/viralloc.c index b8ca850764..e254416cdf 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c @@ -178,7 +178,8 @@ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove) if (virReallocN(ptrptr, size, *countptr -= toremove) < 0) abort(); } else { - virFree(ptrptr); + g_free(*((void **)ptrptr)); + *((void **)ptrptr) = NULL; *countptr = 0; } } @@ -333,24 +334,6 @@ int virAllocVar(void *ptrptr, } -/** - * virFree: - * @ptrptr: pointer to pointer for address of memory to be freed - * - * Release the chunk of memory in the pointer pointed to by - * the 'ptrptr' variable. After release, 'ptrptr' will be - * updated to point to NULL. - */ -void virFree(void *ptrptr) -{ - int save_errno = errno; - - g_free(*(void**)ptrptr); - *(void**)ptrptr = NULL; - errno = save_errno; -} - - /** * virDispose: * @ptrptr: pointer to pointer for address of memory to be sanitized and freed diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 1d42aeead1..833f85f62e 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -55,7 +55,6 @@ int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); int virAllocVar(void *ptrptr, size_t struct_size, size_t element_size, size_t count) G_GNUC_WARN_UNUSED_RESULT ATTRIBUTE_NONNULL(1); -void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); void virDispose(void *ptrptr, size_t count, size_t element_size, size_t *countptr) ATTRIBUTE_NONNULL(1); @@ -417,11 +416,7 @@ void virDisposeString(char **strptr) * * This macro is safe to use on arguments with side effects. */ -/* The ternary ensures that ptr is a non-const pointer and not an - * integer type, all while evaluating ptr only once. This gives us - * extra compiler safety when compiling under gcc. - */ -#define VIR_FREE(ptr) virFree(1 ? (void *) &(ptr) : (ptr)) +#define VIR_FREE(ptr) g_clear_pointer(&(ptr), g_free) /** -- 2.24.1