diff --git a/HACKING b/HACKING index 17ad34479a..4a9516745a 100644 --- a/HACKING +++ b/HACKING @@ -280,9 +280,9 @@ 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 memory.h. -- e.g. to allocate a single object: +- To allocate a single object: virDomainPtr domain; @@ -293,10 +293,10 @@ routines, use the macros from memory.h -- e.g. to allocate an array of objects +- To allocate an array of objects: virDomainPtr domains; - int ndomains = 10; + size_t ndomains = 10; if (VIR_ALLOC_N(domains, ndomains) < 0) { virReportOOMError(); @@ -305,7 +305,7 @@ routines, use the macros from memory.h -- e.g. to allocate an array of object pointers +- To allocate an array of object pointers: virDomainPtr *domains; int ndomains = 10; @@ -317,18 +317,22 @@ routines, use the macros from memory.h -- e.g. to re-allocate the array of domains to be longer +- To re-allocate the array of domains to be longer: - ndomains = 20 - - if (VIR_REALLOC_N(domains, ndomains) < 0) { + if (VIR_EXPAND_N(domains, ndomains, 10) < 0) { virReportOOMError(); return NULL; } -- e.g. to free the domain +- To trim an array of domains to have one less element: + + VIR_SHRINK_N(domains, ndomains, 1); + + + +- To free the domain: VIR_FREE(domain); @@ -344,7 +348,7 @@ code base to help avoiding double-closing of files or file descriptors, which is particulary dangerous in a multi-threaded applications. Instead of these APIs, use the macros from files.h -- eg opening a file from a file descriptor +- Open a file from a file descriptor: if ((file = VIR_FDOPEN(fd, "r")) == NULL) { virReportSystemError(errno, "%s", @@ -355,7 +359,7 @@ APIs, use the macros from files.h -- e.g. close a file descriptor +- Close a file descriptor: if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, "%s", _("failed to close file")); @@ -363,7 +367,7 @@ APIs, use the macros from files.h -- eg close a file +- Close a file: if (VIR_FCLOSE(file) < 0) { virReportSystemError(errno, "%s", _("failed to close file")); @@ -371,8 +375,8 @@ APIs, use the macros from files.h -- eg close a file or file descriptor in an error path, without losing the -previous "errno" value +- Close a file or file descriptor in an error path, without losing the previous +"errno" value: VIR_FORCE_CLOSE(fd); VIR_FORCE_FCLOSE(file); @@ -460,7 +464,7 @@ If there is a need for complex string concatenations, avoid using the usual sequence of malloc/strcpy/strcat/snprintf functions and make use of the virBuffer API described in buf.h -eg typical usage is as follows: +Typical usage is as follows: char * somefunction(...) @@ -594,7 +598,7 @@ When using goto, please use one of these standard labels if it makes sense: error: A path only taken upon return with an error code cleanup: A path taken upon return with success code + optional error no_memory: A path only taken upon return with an OOM error code - retry: If needing to jump upwards (eg retry on EINTR) + retry: If needing to jump upwards (e.g., retry on EINTR) Libvirt committer guidelines diff --git a/docs/hacking.html.in b/docs/hacking.html.in index e1b51856c7..964904e3db 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -354,11 +354,12 @@ 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 memory.h.
e.g. to allocate a single object:
+To allocate a single object:
+virDomainPtr domain; @@ -369,10 +370,10 @@
e.g. to allocate an array of objects
+To allocate an array of objects:
virDomainPtr domains; - int ndomains = 10; + size_t ndomains = 10; if (VIR_ALLOC_N(domains, ndomains) < 0) { virReportOOMError(); @@ -381,7 +382,7 @@
e.g. to allocate an array of object pointers
+To allocate an array of object pointers:
virDomainPtr *domains; int ndomains = 10; @@ -393,18 +394,22 @@
e.g. to re-allocate the array of domains to be longer
+To re-allocate the array of domains to be longer:
- ndomains = 20 - - if (VIR_REALLOC_N(domains, ndomains) < 0) { + if (VIR_EXPAND_N(domains, ndomains, 10) < 0) { virReportOOMError(); return NULL; }
e.g. to free the domain
+To trim an array of domains to have one less element:
+ ++ VIR_SHRINK_N(domains, ndomains, 1); +
To free the domain:
VIR_FREE(domain);@@ -421,7 +426,7 @@
eg opening a file from a file descriptor
+Open a file from a file descriptor:
if ((file = VIR_FDOPEN(fd, "r")) == NULL) { @@ -432,14 +437,14 @@ /* fd is now invalid; only access the file using file variable */
e.g. close a file descriptor
+Close a file descriptor:
if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, "%s", _("failed to close file")); }
eg close a file
+Close a file:
if (VIR_FCLOSE(file) < 0) { @@ -447,8 +452,8 @@ }
eg close a file or file descriptor in an error path, without losing
- the previous errno
value
Close a file or file descriptor in an error path, without losing
+ the previous errno
value:
VIR_FORCE_CLOSE(fd); @@ -554,7 +559,7 @@ make use of the virBuffer API described in buf.h -eg typical usage is as follows:
+Typical usage is as follows:
char * @@ -722,7 +727,7 @@ error: A path only taken upon return with an error code cleanup: A path taken upon return with success code + optional error no_memory: A path only taken upon return with an OOM error code - retry: If needing to jump upwards (eg retry on EINTR) + retry: If needing to jump upwards (e.g., retry on EINTR)diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 50ecebac64..796559c597 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -503,8 +503,10 @@ virLogUnlock; # memory.h virAlloc; virAllocN; +virExpandN; virFree; virReallocN; +virShrinkN; # network.h diff --git a/src/util/memory.c b/src/util/memory.c index dd1216b7d4..59685b30ef 100644 --- a/src/util/memory.c +++ b/src/util/memory.c @@ -1,6 +1,7 @@ /* * memory.c: safer memory allocation * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -24,6 +25,7 @@ #include#include "memory.h" +#include "ignore-value.h" #if TEST_OOM @@ -141,7 +143,7 @@ int virAllocN(void *ptrptr, size_t size, size_t count) * 'count' elements, each 'size' bytes in length. Update 'ptrptr' * with the address of the newly allocated memory. On failure, * 'ptrptr' is not changed and still points to the original memory - * block. The newly allocated memory is filled with zeros. + * block. Any newly allocated memory in 'ptrptr' is uninitialized. * * Returns -1 on failure to allocate, zero on success */ @@ -164,6 +166,61 @@ int virReallocN(void *ptrptr, size_t size, size_t count) return 0; } +/** + * virExpandN: + * @ptrptr: pointer to pointer for address of allocated memory + * @size: number of bytes per element + * @countptr: pointer to number of elements in array + * @add: number of elements to add + * + * Resize the block of memory in 'ptrptr' to be an array of + * '*countptr' + 'add' elements, each 'size' bytes in length. + * Update 'ptrptr' and 'countptr' with the details of the newly + * allocated memory. On failure, 'ptrptr' and 'countptr' are not + * changed. Any newly allocated memory in 'ptrptr' is zero-filled. + * + * Returns -1 on failure to allocate, zero on success + */ +int virExpandN(void *ptrptr, size_t size, size_t *countptr, size_t add) +{ + int ret; + + if (*countptr + add < *countptr) { + errno = ENOMEM; + return -1; + } + ret = virReallocN(ptrptr, size, *countptr + add); + if (ret == 0) { + memset(*(char **)ptrptr + (size * *countptr), 0, size * add); + *countptr += add; + } + return ret; +} + +/** + * virShrinkN: + * @ptrptr: pointer to pointer for address of allocated memory + * @size: number of bytes per element + * @countptr: pointer to number of elements in array + * @remove: number of elements to remove + * + * Resize the block of memory in 'ptrptr' to be an array of + * '*countptr' - 'remove' elements, each 'size' bytes in length. + * Update 'ptrptr' and 'countptr' with the details of the newly + * allocated memory. If 'remove' is larger than 'countptr', free + * the entire array. + */ +void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t remove) +{ + if (remove < *countptr) + ignore_value(virReallocN(ptrptr, size, *countptr -= remove)); + else { + virFree(ptrptr); + *countptr = 0; + } +} + + /** * Vir_Alloc_Var: * @ptrptr: pointer to hold address of allocated memory diff --git a/src/util/memory.h b/src/util/memory.h index 60e2be66f8..98ac2b3582 100644 --- a/src/util/memory.h +++ b/src/util/memory.h @@ -46,14 +46,21 @@ /* Don't call these directly - use the macros below */ -int virAlloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK; -int virAllocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK; -int virReallocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK; +int virAlloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK + ATTRIBUTE_NONNULL(1); +int virAllocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK + ATTRIBUTE_NONNULL(1); +int virReallocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK + ATTRIBUTE_NONNULL(1); +int virExpandN(void *ptrptr, size_t size, size_t *count, size_t add) + ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); +void virShrinkN(void *ptrptr, size_t size, size_t *count, size_t remove) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); int virAllocVar(void *ptrptr, size_t struct_size, size_t element_size, - size_t count) ATTRIBUTE_RETURN_CHECK; -void virFree(void *ptrptr); + size_t count) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1); +void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); /** * VIR_ALLOC: @@ -87,12 +94,44 @@ void virFree(void *ptrptr); * * Re-allocate an array of 'count' elements, each sizeof(*ptr) * bytes long and store the address of allocated memory in - * 'ptr'. Fill the newly allocated memory with zeros + * 'ptr'. If 'ptr' grew, the added memory is uninitialized. * * Returns -1 on failure, 0 on success */ # define VIR_REALLOC_N(ptr, count) virReallocN(&(ptr), sizeof(*(ptr)), (count)) +/** + * VIR_EXPAND_N: + * @ptr: pointer to hold address of allocated memory + * @count: variable tracking number of elements currently allocated + * @add: number of elements to add + * + * Re-allocate an array of 'count' elements, each sizeof(*ptr) + * bytes long, to be 'count' + 'add' elements long, then store the + * address of allocated memory in 'ptr' and the new size in 'count'. + * The new elements are filled with zero. + * + * Returns -1 on failure, 0 on success + */ +# define VIR_EXPAND_N(ptr, count, add) \ + virExpandN(&(ptr), sizeof(*(ptr)), &(count), add) + +/** + * VIR_SHRINK_N: + * @ptr: pointer to hold address of allocated memory + * @count: variable tracking number of elements currently allocated + * @remove: number of elements to remove + * + * Re-allocate an array of 'count' elements, each sizeof(*ptr) + * bytes long, to be 'count' - 'remove' elements long, then store the + * address of allocated memory in 'ptr' and the new size in 'count'. + * If 'count' <= 'remove', the entire array is freed. + * + * No return value. + */ +# define VIR_SHRINK_N(ptr, count, remove) \ + virShrinkN(&(ptr), sizeof(*(ptr)), &(count), remove) + /* * VIR_ALLOC_VAR_OVERSIZED: * @M: size of base structure