[libvirt] [PATCH 4/4] virFree: Check const correctness
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(a)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