Now that we've finally fixed all the violators, it's time to
enforce that any pointer to a const object is never freed (it
is aliasing some other memory, where the non-const original
should be freed instead). Alas, the code still needs a normal
vs. Coverity version, but at least we are still guaranteeing
that the macro call evaluates its argument exactly once.
I verified that we still get the following compiler warnings,
which in turn halts the build thanks to -Werror on gcc (hmm,
gcc 4.8.3's placement of the ^ for ?: type mismatch is a bit
off, but that's not our problem):
int oops1 = 0;
VIR_FREE(oops1);
const char *oops2 = NULL;
VIR_FREE(oops2);
struct blah { int dummy; } oops3;
VIR_FREE(oops3);
util/virauthconfig.c:159:35: error: pointer/integer type mismatch in conditional expression [-Werror]
VIR_FREE(oops1);
^
util/virauthconfig.c:161:5: error: passing argument 1 of 'virFree' discards 'const' qualifier from pointer target type [-Werror]
VIR_FREE(oops2);
^
In file included from util/virauthconfig.c:28:0:
util/viralloc.h:79:6: note: expected 'void *' but argument is of type 'const void *'
void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
^
util/virauthconfig.c:163:35: error: type mismatch in conditional expression
VIR_FREE(oops3);
^
* src/util/viralloc.h (VIR_FREE): No longer cast away const.
* src/xenapi/xenapi_utils.c (xenSessionFree): Work around bogus
header.
Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
v2: this depends on the existing 1/4, while being a replacement
to all of 2-4/4 at once.
https://www.redhat.com/archives/libvir-list/2014-July/msg00716.html
src/util/viralloc.h | 11 +++++------
src/xenapi/xenapi_utils.c | 4 +++-
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 7125e67..bf85c16 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -548,18 +548,17 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* This macro is safe to use on arguments with side effects.
*/
# if !STATIC_ANALYSIS
-/* The ternary ensures that ptr is a pointer and not an integer type,
- * while evaluating ptr only once. This gives us extra compiler
- * safety when compiling under gcc. For now, we intentionally cast
- * away const, since a number of callers safely pass const char *.
+/* 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((void *) (1 ? (const void *) &(ptr) : (ptr)))
+# define VIR_FREE(ptr) virFree(1 ? (void *) &(ptr) : (ptr))
# else
/* The Coverity static analyzer considers the else path of the "?:" and
* flags the VIR_FREE() of the address of the address of memory as a
* RESOURCE_LEAK resulting in numerous false positives (eg, VIR_FREE(&ptr))
*/
-# define VIR_FREE(ptr) virFree((void *) &(ptr))
+# define VIR_FREE(ptr) virFree(&(ptr))
# endif
void virAllocTestInit(void);
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index a80d136..ef89f42 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -44,13 +44,15 @@ void
xenSessionFree(xen_session *session)
{
size_t i;
+ char *tmp;
if (session->error_description != NULL) {
for (i = 0; i < session->error_description_count; i++)
VIR_FREE(session->error_description[i]);
VIR_FREE(session->error_description);
}
/* The session_id member is type of 'const char *'. Sigh. */
- VIR_FREE(session->session_id);
+ tmp = (char *)session->session_id;
+ VIR_FREE(tmp);
VIR_FREE(session);
}
--
1.9.3
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list