[libvirt] [PATCH 4/4] virFree: Check const correctness

[Michal Privoznik]

Up to now it's possible to do something like this:

const char *ptr;

ptr = strdup("my example string");

VIR_FREE(ptr);

The problem is, const char * pointers should not be modified (and
freeing them is kind of modification). We should avoid this. A little
trick is used: assigning a const pointer into 'void *' triggers
compiler warning about discarding 'const' qualifier from pointer. So
the virFree() function gains new dummy argument, that is not touched
anyhow, just fulfills the const correctness check duty.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/util/viralloc.c       |  6 ++++--
 src/util/viralloc.h       | 20 ++++++++++++++++----
 src/xenapi/xenapi_utils.c |  2 +-
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/util/viralloc.c b/src/util/viralloc.c
index be9f0fe..0134e67 100644
--- a/src/util/viralloc.c
+++ b/src/util/viralloc.c
@@ -372,7 +372,7 @@ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove)
         ignore_value(virReallocN(ptrptr, size, *countptr -= toremove,
                                  false, 0, NULL, NULL, 0));
     else {
-        virFree(ptrptr);
+        virFree(NULL, ptrptr);
         *countptr = 0;
     }
 }
@@ -569,13 +569,15 @@ int virAllocVar(void *ptrptr,
 
 /**
  * virFree:
+ * @ptr: dummy pointer to check const correctness
  * @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)
+void virFree(void *ptr ATTRIBUTE_UNUSED,
+             void *ptrptr)
 {
     int save_errno = errno;
 
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 71b4a45..8fbe56f 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -76,7 +76,7 @@ int virAllocVar(void *ptrptr, size_t struct_size, size_t element_size, size_t co
                 bool report, int domcode, const char *filename,
                 const char *funcname, size_t linenr)
     ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1);
-void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
+void virFree(void *ptr, void *ptrptr) ATTRIBUTE_NONNULL(2);
 
 /**
  * VIR_ALLOC:
@@ -543,11 +543,23 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
  * @ptr: pointer holding address to be freed
  *
  * Free the memory stored in 'ptr' and update to point
- * to NULL.
+ * to NULL. Moreover, this macro has a side effect in
+ * form of evaluating passed argument multiple times.
+ * But advantage is, it checks the cont correctness
+ * (that you are not freeing a const pointer).
+ */
+# define VIR_FREE(ptr) virFree(ptr, &(ptr))
+
+/**
+ * VIR_FREE_BROKEN:
+ * @ptr: pointer holding address to be freed
  *
- * This macro is safe to use on arguments with side effects.
+ * Twin macro of VIR_FREE. While it does evaluate
+ * argument only once, it does not check const
+ * correctness and therefore you want to use it if and
+ * only if necessary.
  */
-# define VIR_FREE(ptr) virFree(&(ptr))
+# define VIR_FREE_BROKEN(ptr) virFree(NULL, &(ptr))
 
 void virAllocTestInit(void);
 int virAllocTestCount(void);
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index a80d136..db555d2 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -50,7 +50,7 @@ xenSessionFree(xen_session *session)
         VIR_FREE(session->error_description);
     }
     /* The session_id member is type of 'const char *'. Sigh. */
-    VIR_FREE(session->session_id);
+    VIR_FREE_BROKEN(session->session_id);
     VIR_FREE(session);
 }
 
-- 
1.8.5.5