On Sat, Aug 14, 2010 at 10:23:53AM -0600, Eric Blake wrote: > Note - I did _not_ update HACKING. Do we have a makefile rule > for doing an auto-xsl translation from hacking.html.in yet, or > is something I'll have to do manually before pushing this? > > * src/util/memory.h (VIR_REALLOC_N): Update docs. > (VIR_EXPAND_N, VIR_SHRINK_N): New macros. > (virAlloc, virAllocN, virReallocN, virAllocVar, virFree): Add some > gcc attributes. > * src/util/memory.c (virExpandN, virShrinkN): New functions. > (virReallocN): Update docs. > * docs/hacking.html.in: Prefer newer interfaces over > VIR_REALLOC_N, since uninitialized memory can bite us. > --- > docs/hacking.html.in | 24 +++++++++++-------- > src/util/memory.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++- > src/util/memory.h | 51 ++++++++++++++++++++++++++++++++++++++----- > 3 files changed, 117 insertions(+), 17 deletions(-) > > diff --git a/docs/hacking.html.in b/docs/hacking.html.in > index deab530..4cc92f4 100644 > --- a/docs/hacking.html.in > +++ b/docs/hacking.html.in > @@ -332,11 +332,11 @@ > 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. > </p> > > <ul> > - <li><p>eg to allocate a single object:</p> > + <li><p>To allocate a single object:</p> > > <pre> > virDomainPtr domain; > @@ -347,11 +347,11 @@ > } > </pre></li> > > - <li><p>eg to allocate an array of objects</p> > + <li><p>To allocate an array of objects</p> > > <pre> > virDomainPtr domains; > - int ndomains = 10; > + size_t ndomains = 10; > > if (VIR_ALLOC_N(domains, ndomains) < 0) { > virReportOOMError(); > @@ -359,7 +359,7 @@ > } > </pre></li> > > - <li><p>eg to allocate an array of object pointers</p> > + <li><p>To allocate an array of object pointers</p> > > <pre> > virDomainPtr *domains; > @@ -371,18 +371,22 @@ > } > </pre></li> > > - <li><p>eg to re-allocate the array of domains to be longer</p> > + <li><p>To re-allocate the array of domains to be 10 elements longer</p> > > <pre> > - ndomains = 20 > - > - if (VIR_REALLOC_N(domains, ndomains) < 0) { > + if (VIR_EXPAND_N(domains, ndomains, 10) < 0) { > virReportOOMError(); > return NULL; > } > </pre></li> > > - <li><p>eg to free the domain</p> > + <li><p>To trim an array of domains to have one less element</p> > + > +<pre> > + VIR_SHRINK_N(domains, ndomains, 1); > +</pre></li> > + > + <li><p>To free the domain</p> > > <pre> > VIR_FREE(domain); > diff --git a/src/util/memory.c b/src/util/memory.c > index dd1216b..e95aa47 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 <stddef.h> > > #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 > */ > @@ -165,6 +167,61 @@ int virReallocN(void *ptrptr, size_t size, size_t count) > } > > /** > + * 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 res; > + > + if (*countptr + add < *countptr) { > + errno = ENOMEM; > + return -1; > + } > + res = virReallocN(ptrptr, size, *countptr + add); > + if (res == 0) { > + memset(*(char **)ptrptr + (size * *countptr), 0, size * add); > + *countptr += add; > + } > + return res; > +} > + > +/** > + * 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 > * @struct_size: size of initial struct > diff --git a/src/util/memory.h b/src/util/memory.h > index 60e2be6..98ac2b3 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 ACK, I've wanted to add this for a long time now Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list