[PATCH 5/5] VIR_ALLOC: Replace internals by g_clear_pointer

Peter Krempa pkrempa at redhat.com
Thu Mar 5 09:52:02 UTC 2020


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 at redhat.com>
---
 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




More information about the libvir-list mailing list