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@xxxxxxxxxx> --- 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