External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-07-17 07:07:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

alloc: make VIR_APPEND_ELEMENT safer

Browse Source 
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 <eblake@redhat.com>
This commit is contained in:
Eric Blake 2013-05-06 16:21:15 -06:00
parent 605a077244
commit ddcfc5492a
4 changed files with 47 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
HACKING
View File

@ -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:

9
docs/hacking.html.in
View File

@ -519,6 +519,13 @@
<h2><a name="preprocessor">Preprocessor</a></h2>
<p>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.
</p>
<p>
For variadic macros, stick with C99 syntax:
</p>
@ -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.
</p>
<ul>

9
src/util/viralloc.c
View File

@ -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;

36
src/util/viralloc.h
View File

@ -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,