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 @@
+ 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.
diff --git a/src/util/viralloc.c b/src/util/viralloc.c
index 8f219bf1b5..8d6a7e6d32 100644
--- a/src/util/viralloc.c
+++ b/src/util/viralloc.c
@@ -281,7 +281,7 @@ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove)
* virInsertElementsN:
* @ptrptr: pointer to hold address of allocated memory
* @size: the size of one element in bytes
- * @at: index within array where new elements should be added
+ * @at: index within array where new elements should be added, -1 for end
* @countptr: variable tracking number of elements currently allocated
* @add: number of elements to add
* @newelems: pointer to array of one or more new elements to move into
@@ -300,7 +300,8 @@ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove)
* items from newelems into ptrptr[at], then store the address of
* allocated memory in *ptrptr and the new size in *countptr. If
* newelems is NULL, the new elements at ptrptr[at] are instead filled
- * with zero.
+ * with zero. at must be between [0,*countptr], except that -1 is
+ * treated the same as *countptr for convenience.
*
* Returns -1 on failure, 0 on success
*/
@@ -310,7 +311,9 @@ virInsertElementsN(void *ptrptr, size_t size, size_t at,
size_t add, void *newelems,
bool clearOriginal, bool inPlace)
{
- if (at > *countptr) {
+ if (at == -1) {
+ at = *countptr;
+ } else if (at > *countptr) {
VIR_WARN("out of bounds index - count %zu at %zu add %zu",
*countptr, at, add);
return -1;
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 7be7f82d57..35d3a37f74 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -80,6 +80,8 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* the address of allocated memory in 'ptr'. Fill the
* newly allocated memory with zeros.
*
+ * This macro is safe to use on arguments with side effects.
+ *
* Returns -1 on failure, 0 on success
*/
# define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr)))
@@ -93,6 +95,8 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* bytes long and store the address of allocated memory in
* 'ptr'. Fill the newly allocated memory with zeros.
*
+ * This macro is safe to use on arguments with side effects.
+ *
* Returns -1 on failure, 0 on success
*/
# define VIR_ALLOC_N(ptr, count) virAllocN(&(ptr), sizeof(*(ptr)), (count))
@@ -106,6 +110,8 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* bytes long and store the address of allocated memory in
* 'ptr'. If 'ptr' grew, the added memory is uninitialized.
*
+ * This macro is safe to use on arguments with side effects.
+ *
* Returns -1 on failure, 0 on success
*/
# define VIR_REALLOC_N(ptr, count) virReallocN(&(ptr), sizeof(*(ptr)), (count))
@@ -121,6 +127,8 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* address of allocated memory in 'ptr' and the new size in 'count'.
* The new elements are filled with zero.
*
+ * This macro is safe to use on arguments with side effects.
+ *
* Returns -1 on failure, 0 on success
*/
# define VIR_EXPAND_N(ptr, count, add) \
@@ -144,6 +152,8 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* the address of allocated memory in 'ptr' and the new size in 'alloc'.
* The new elements are filled with zero.
*
+ * This macro is safe to use on arguments with side effects.
+ *
* Returns -1 on failure, 0 on success
*/
# define VIR_RESIZE_N(ptr, alloc, count, add) \
@@ -160,6 +170,8 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* address of allocated memory in 'ptr' and the new size in 'count'.
* If 'count' <= 'remove', the entire array is freed.
*
+ * This macro is safe to use on arguments with side effects.
+ *
* No return value.
*/
# define VIR_SHRINK_N(ptr, count, remove) \
@@ -236,9 +248,9 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* replacing it with VIR_TYPECHECK(ptr, &newelem) so that we can be
* assured ptr and &newelem are of compatible types.
*
+ * These macros are safe to use on arguments with side effects.
+ *
* Returns -1 on failure, 0 on success
- *
- *
*/
# define VIR_INSERT_ELEMENT(ptr, at, count, newelem) \
virInsertElementsN(&(ptr), sizeof(*(ptr)), at, &(count), \
@@ -284,21 +296,21 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* replacing it with VIR_TYPECHECK(ptr, &newelem) so that we can be
* assured ptr and &newelem are of compatible types.
*
+ * These macros are safe to use on arguments with side effects.
+ *
* Returns -1 on failure, 0 on success
- *
- *
*/
# define VIR_APPEND_ELEMENT(ptr, count, newelem) \
- virInsertElementsN(&(ptr), sizeof(*(ptr)), count, &(count), \
+ virInsertElementsN(&(ptr), sizeof(*(ptr)), -1, &(count), \
VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, false)
# define VIR_APPEND_ELEMENT_COPY(ptr, count, newelem) \
- virInsertElementsN(&(ptr), sizeof(*(ptr)), count, &(count), \
+ virInsertElementsN(&(ptr), sizeof(*(ptr)), -1, &(count), \
VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, false)
# define VIR_APPEND_ELEMENT_INPLACE(ptr, count, newelem) \
- virInsertElementsN(&(ptr), sizeof(*(ptr)), count, &(count), \
+ virInsertElementsN(&(ptr), sizeof(*(ptr)), -1, &(count), \
VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, true)
# define VIR_APPEND_ELEMENT_COPY_INPLACE(ptr, count, newelem) \
- virInsertElementsN(&(ptr), sizeof(*(ptr)), count, &(count), \
+ virInsertElementsN(&(ptr), sizeof(*(ptr)), -1, &(count), \
VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, true)
/**
@@ -315,6 +327,8 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* VIR_DELETE_ELEMENT_INPLACE is identical, but assumes any
* necessary memory re-allocation will be done later.
*
+ * These macros are safe to use on arguments with side effects.
+ *
* Returns -1 on failure, 0 on success
*/
# define VIR_DELETE_ELEMENT(ptr, at, count) \
@@ -348,7 +362,9 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* that will be returned and allocate a suitable buffer to contain the
* returned data. C99 refers to these variable length objects as
* structs containing flexible array members.
-
+ *
+ * This macro is safe to use on arguments with side effects.
+ *
* Returns -1 on failure, 0 on success
*/
# define VIR_ALLOC_VAR(ptr, type, count) \
@@ -360,6 +376,8 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
*
* Free the memory stored in 'ptr' and update to point
* to NULL.
+ *
+ * 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,