From ddcfc5492a118624793a6ed48ee9afb5769fc0fb Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 6 May 2013 16:21:15 -0600 Subject: [PATCH] alloc: make VIR_APPEND_ELEMENT safer VIR_APPEND_ELEMENT(array, size, elem) was not safe if the expression for 'size' had side effects. While no one in the current code base was trying to pass side effects, we might as well be robust and explicitly document our intentions. * src/util/viralloc.c (virInsertElementsN): Add special case. * src/util/viralloc.h (VIR_APPEND_ELEMENT): Use it. (VIR_ALLOC, VIR_ALLOC_N, VIR_REALLOC_N, VIR_EXPAND_N) (VIR_RESIZE_N, VIR_SHRINK_N, VIR_INSERT_ELEMENT) (VIR_DELETE_ELEMENT, VIR_ALLOC_VAR, VIR_FREE): Document which macros are safe in the presence of side effects. * docs/hacking.html.in: Document this. * HACKING: Regenerate. Signed-off-by: Eric Blake --- HACKING | 7 ++++++- docs/hacking.html.in | 9 ++++++++- src/util/viralloc.c | 9 ++++++--- src/util/viralloc.h | 36 +++++++++++++++++++++++++++--------- 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/HACKING b/HACKING index b70b89d016..b4dcd3da62 100644 --- a/HACKING +++ b/HACKING @@ -418,6 +418,11 @@ But if negating a complex condition is too ugly, then at least add braces: Preprocessor ============ +Macros defined with an ALL_CAPS name should generally be assumed to be unsafe +with regards to arguments with side-effects (that is, MAX(a++, b--) might +increment a or decrement b too many or too few times). Exceptions to this rule +are explicitly documented for macros in viralloc.h. + For variadic macros, stick with C99 syntax: #define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__) @@ -501,7 +506,7 @@ Low level memory management Use of the malloc/free/realloc/calloc APIs is deprecated in the libvirt codebase, because they encourage a number of serious coding bugs and do not enable compile time verification of checks for NULL. Instead of these -routines, use the macros from memory.h. +routines, use the macros from viralloc.h. - To allocate a single object: diff --git a/docs/hacking.html.in b/docs/hacking.html.in index b15d18711c..8708cb3a72 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -519,6 +519,13 @@

Preprocessor

+

Macros defined with an ALL_CAPS name should generally be + assumed to be unsafe with regards to arguments with side-effects + (that is, MAX(a++, b--) might increment a or decrement b too + many or too few times). Exceptions to this rule are explicitly + documented for macros in viralloc.h. +

+

For variadic macros, stick with C99 syntax:

@@ -616,7 +623,7 @@ Use of the malloc/free/realloc/calloc APIs is deprecated in the libvirt codebase, because they encourage a number of serious coding bugs and do not enable compile time verification of checks for NULL. Instead of these - routines, use the macros from memory.h. + routines, use the macros from viralloc.h.