Re: [libvirt] [PATCH v3 03/19] util: use glib memory allocation functions
Not reproduced when build source before `make clean`. Please ignore that issue On Tue, Oct 15, 2019 at 10:20 AM Han Han wrote: > > > On Thu, Oct 10, 2019 at 6:54 PM Daniel P. Berrangé > wrote: > >> Convert the VIR_ALLOC family of APIs with use of the g_malloc family of >> APIs. Use of VIR_ALLOC related functions should be incrementally phased >> out over time, allowing return value checks to be dropped. Use of >> VIR_FREE should be replaced with auto-cleanup whenever possible. >> >> We previously used the 'calloc-posix' gnulib module because mingw does >> not set errno to ENOMEM on failure. >> >> Reviewed-by: Ján Tomko >> Signed-off-by: Daniel P. Berrangé >> --- >> bootstrap.conf | 1 - >> docs/hacking.html.in | 106 +-- >> src/util/viralloc.c | 29 +++- >> src/util/viralloc.h | 9 >> 4 files changed, 27 insertions(+), 118 deletions(-) >> >> diff --git a/bootstrap.conf b/bootstrap.conf >> index 358d783a6b..7d73584809 100644 >> --- a/bootstrap.conf >> +++ b/bootstrap.conf >> @@ -26,7 +26,6 @@ byteswap >> c-ctype >> c-strcase >> c-strcasestr >> -calloc-posix >> canonicalize-lgpl >> chown >> clock-time >> diff --git a/docs/hacking.html.in b/docs/hacking.html.in >> index 2e064ced5e..8072796312 100644 >> --- a/docs/hacking.html.in >> +++ b/docs/hacking.html.in >> @@ -1008,102 +1008,20 @@ BAD: >> >> >> >> + VIR_ALLOC, VIR_REALLOC, VIR_RESIZE_N, VIR_EXPAND_N, >> +VIR_SHRINK_N, VIR_FREE, VIR_APPEND_ELEMENT, VIR_INSERT_ELEMENT, >> +VIR_DELETE_ELEMENT >> + Prefer the GLib APIs g_new0/g_renew/g_free in most cases. >> +There should rarely be a need to use g_malloc/g_realloc. >> +Instead of using plain C arrays, it is preferrable to use >> +one of the GLib types, GArray, GPtrArray or GByteArray. These >> +all use a struct to track the array memory and size together >> +and efficiently resize. NEVER MIX use of the >> +classic libvirt memory allocation APIs and GLib APIs within >> +a single method. Keep the style consistent, converting existing >> +code to GLib style in a separate, prior commit. >> >> >> -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 viralloc.h. >> - >> - >> - >> - To allocate a single object: >> - >> - >> - virDomainPtr domain; >> - >> - if (VIR_ALLOC(domain) < 0) >> - return NULL; >> - >> - >> - >> - To allocate an array of objects: >> - >> - virDomainPtr domains; >> - size_t ndomains = 10; >> - >> - if (VIR_ALLOC_N(domains, ndomains) < 0) >> - return NULL; >> - >> - >> - >> - To allocate an array of object pointers: >> - >> - virDomainPtr *domains; >> - size_t ndomains = 10; >> - >> - if (VIR_ALLOC_N(domains, ndomains) < 0) >> - return NULL; >> - >> - >> - >> - To re-allocate the array of domains to be 1 element >> - longer (however, note that repeatedly expanding an array by 1 >> - scales quadratically, so this is recommended only for smaller >> - arrays): >> - >> - virDomainPtr domains; >> - size_t ndomains = 0; >> - >> - if (VIR_EXPAND_N(domains, ndomains, 1) < 0) >> - return NULL; >> - domains[ndomains - 1] = domain; >> - >> - >> - To ensure an array has room to hold at least one more >> - element (this approach scales better, but requires tracking >> - allocation separately from usage) >> - >> - >> - virDomainPtr domains; >> - size_t ndomains = 0; >> - size_t ndomains_max = 0; >> - >> - if (VIR_RESIZE_N(domains, ndomains_max, ndomains, 1) < 0) >> - return NULL; >> - domains[ndomains++] = domain; >> - >> - >> - >> - To trim an array of domains from its allocated size down >> - to the actual used size: >> - >> - >> - virDomainPtr domains; >> - size_t ndomains = x; >> - size_t ndomains_max = y; >> - >> - VIR_SHRINK_N(domains, ndomains_max, ndomains_max - ndomains); >> - >> - >> - To free an array of domains: >> - >> - virDomainPtr domains; >> - size_t ndomains = x; >> - size_t ndomains_max = y; >> - size_t i; >> - >> - for (i = 0; i < ndomains; i++) >> - VIR_FREE(domains[i]); >> - VIR_FREE(domains); >> - ndomains_max = ndomains = 0; >> - >> - >> - >> - >> File handling >> >> >> diff --git a/src/util/viralloc.c b/src/util/viralloc.c >> index 10a8d0fb73..b8ca850764 100644 >> --- a/src/util/viralloc.c >> +++ b/src/util/viralloc.c >> @@ -45,10 +45,7 @@ VIR_LOG_INIT("util.alloc"); >> int virAlloc(void *ptrptr, >> size_t size) >> { >> -*(void **)ptrptr = calloc(1, size); >> -if (*(void **)ptrptr == NULL) >> -abort(); >> - >> +
Re: [libvirt] [PATCH v3 03/19] util: use glib memory allocation functions
On Thu, Oct 10, 2019 at 6:54 PM Daniel P. Berrangé wrote: > Convert the VIR_ALLOC family of APIs with use of the g_malloc family of > APIs. Use of VIR_ALLOC related functions should be incrementally phased > out over time, allowing return value checks to be dropped. Use of > VIR_FREE should be replaced with auto-cleanup whenever possible. > > We previously used the 'calloc-posix' gnulib module because mingw does > not set errno to ENOMEM on failure. > > Reviewed-by: Ján Tomko > Signed-off-by: Daniel P. Berrangé > --- > bootstrap.conf | 1 - > docs/hacking.html.in | 106 +-- > src/util/viralloc.c | 29 +++- > src/util/viralloc.h | 9 > 4 files changed, 27 insertions(+), 118 deletions(-) > > diff --git a/bootstrap.conf b/bootstrap.conf > index 358d783a6b..7d73584809 100644 > --- a/bootstrap.conf > +++ b/bootstrap.conf > @@ -26,7 +26,6 @@ byteswap > c-ctype > c-strcase > c-strcasestr > -calloc-posix > canonicalize-lgpl > chown > clock-time > diff --git a/docs/hacking.html.in b/docs/hacking.html.in > index 2e064ced5e..8072796312 100644 > --- a/docs/hacking.html.in > +++ b/docs/hacking.html.in > @@ -1008,102 +1008,20 @@ BAD: > > > > + VIR_ALLOC, VIR_REALLOC, VIR_RESIZE_N, VIR_EXPAND_N, > +VIR_SHRINK_N, VIR_FREE, VIR_APPEND_ELEMENT, VIR_INSERT_ELEMENT, > +VIR_DELETE_ELEMENT > + Prefer the GLib APIs g_new0/g_renew/g_free in most cases. > +There should rarely be a need to use g_malloc/g_realloc. > +Instead of using plain C arrays, it is preferrable to use > +one of the GLib types, GArray, GPtrArray or GByteArray. These > +all use a struct to track the array memory and size together > +and efficiently resize. NEVER MIX use of the > +classic libvirt memory allocation APIs and GLib APIs within > +a single method. Keep the style consistent, converting existing > +code to GLib style in a separate, prior commit. > > > -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 viralloc.h. > - > - > - > - To allocate a single object: > - > - > - virDomainPtr domain; > - > - if (VIR_ALLOC(domain) < 0) > - return NULL; > - > - > - > - To allocate an array of objects: > - > - virDomainPtr domains; > - size_t ndomains = 10; > - > - if (VIR_ALLOC_N(domains, ndomains) < 0) > - return NULL; > - > - > - > - To allocate an array of object pointers: > - > - virDomainPtr *domains; > - size_t ndomains = 10; > - > - if (VIR_ALLOC_N(domains, ndomains) < 0) > - return NULL; > - > - > - > - To re-allocate the array of domains to be 1 element > - longer (however, note that repeatedly expanding an array by 1 > - scales quadratically, so this is recommended only for smaller > - arrays): > - > - virDomainPtr domains; > - size_t ndomains = 0; > - > - if (VIR_EXPAND_N(domains, ndomains, 1) < 0) > - return NULL; > - domains[ndomains - 1] = domain; > - > - > - To ensure an array has room to hold at least one more > - element (this approach scales better, but requires tracking > - allocation separately from usage) > - > - > - virDomainPtr domains; > - size_t ndomains = 0; > - size_t ndomains_max = 0; > - > - if (VIR_RESIZE_N(domains, ndomains_max, ndomains, 1) < 0) > - return NULL; > - domains[ndomains++] = domain; > - > - > - > - To trim an array of domains from its allocated size down > - to the actual used size: > - > - > - virDomainPtr domains; > - size_t ndomains = x; > - size_t ndomains_max = y; > - > - VIR_SHRINK_N(domains, ndomains_max, ndomains_max - ndomains); > - > - > - To free an array of domains: > - > - virDomainPtr domains; > - size_t ndomains = x; > - size_t ndomains_max = y; > - size_t i; > - > - for (i = 0; i < ndomains; i++) > - VIR_FREE(domains[i]); > - VIR_FREE(domains); > - ndomains_max = ndomains = 0; > - > - > - > - > File handling > > > diff --git a/src/util/viralloc.c b/src/util/viralloc.c > index 10a8d0fb73..b8ca850764 100644 > --- a/src/util/viralloc.c > +++ b/src/util/viralloc.c > @@ -45,10 +45,7 @@ VIR_LOG_INIT("util.alloc"); > int virAlloc(void *ptrptr, > size_t size) > { > -*(void **)ptrptr = calloc(1, size); > -if (*(void **)ptrptr == NULL) > -abort(); > - > +*(void **)ptrptr = g_malloc0(size); > return 0; > } > > @@ -69,10 +66,7 @@ int virAllocN(void *ptrptr, >size_t size, >size_t count) > { > -*(void**)ptrptr = calloc(count, size); > -if (*(void**)ptrptr == NULL) > -abort(); > - > +*(void**)pt
[libvirt] [PATCH v3 03/19] util: use glib memory allocation functions
Convert the VIR_ALLOC family of APIs with use of the g_malloc family of APIs. Use of VIR_ALLOC related functions should be incrementally phased out over time, allowing return value checks to be dropped. Use of VIR_FREE should be replaced with auto-cleanup whenever possible. We previously used the 'calloc-posix' gnulib module because mingw does not set errno to ENOMEM on failure. Reviewed-by: Ján Tomko Signed-off-by: Daniel P. Berrangé --- bootstrap.conf | 1 - docs/hacking.html.in | 106 +-- src/util/viralloc.c | 29 +++- src/util/viralloc.h | 9 4 files changed, 27 insertions(+), 118 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 358d783a6b..7d73584809 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -26,7 +26,6 @@ byteswap c-ctype c-strcase c-strcasestr -calloc-posix canonicalize-lgpl chown clock-time diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 2e064ced5e..8072796312 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -1008,102 +1008,20 @@ BAD: + VIR_ALLOC, VIR_REALLOC, VIR_RESIZE_N, VIR_EXPAND_N, +VIR_SHRINK_N, VIR_FREE, VIR_APPEND_ELEMENT, VIR_INSERT_ELEMENT, +VIR_DELETE_ELEMENT + Prefer the GLib APIs g_new0/g_renew/g_free in most cases. +There should rarely be a need to use g_malloc/g_realloc. +Instead of using plain C arrays, it is preferrable to use +one of the GLib types, GArray, GPtrArray or GByteArray. These +all use a struct to track the array memory and size together +and efficiently resize. NEVER MIX use of the +classic libvirt memory allocation APIs and GLib APIs within +a single method. Keep the style consistent, converting existing +code to GLib style in a separate, prior commit. -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 viralloc.h. - - - - To allocate a single object: - - - virDomainPtr domain; - - if (VIR_ALLOC(domain) < 0) - return NULL; - - - - To allocate an array of objects: - - virDomainPtr domains; - size_t ndomains = 10; - - if (VIR_ALLOC_N(domains, ndomains) < 0) - return NULL; - - - - To allocate an array of object pointers: - - virDomainPtr *domains; - size_t ndomains = 10; - - if (VIR_ALLOC_N(domains, ndomains) < 0) - return NULL; - - - - To re-allocate the array of domains to be 1 element - longer (however, note that repeatedly expanding an array by 1 - scales quadratically, so this is recommended only for smaller - arrays): - - virDomainPtr domains; - size_t ndomains = 0; - - if (VIR_EXPAND_N(domains, ndomains, 1) < 0) - return NULL; - domains[ndomains - 1] = domain; - - - To ensure an array has room to hold at least one more - element (this approach scales better, but requires tracking - allocation separately from usage) - - - virDomainPtr domains; - size_t ndomains = 0; - size_t ndomains_max = 0; - - if (VIR_RESIZE_N(domains, ndomains_max, ndomains, 1) < 0) - return NULL; - domains[ndomains++] = domain; - - - - To trim an array of domains from its allocated size down - to the actual used size: - - - virDomainPtr domains; - size_t ndomains = x; - size_t ndomains_max = y; - - VIR_SHRINK_N(domains, ndomains_max, ndomains_max - ndomains); - - - To free an array of domains: - - virDomainPtr domains; - size_t ndomains = x; - size_t ndomains_max = y; - size_t i; - - for (i = 0; i < ndomains; i++) - VIR_FREE(domains[i]); - VIR_FREE(domains); - ndomains_max = ndomains = 0; - - - - File handling diff --git a/src/util/viralloc.c b/src/util/viralloc.c index 10a8d0fb73..b8ca850764 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c @@ -45,10 +45,7 @@ VIR_LOG_INIT("util.alloc"); int virAlloc(void *ptrptr, size_t size) { -*(void **)ptrptr = calloc(1, size); -if (*(void **)ptrptr == NULL) -abort(); - +*(void **)ptrptr = g_malloc0(size); return 0; } @@ -69,10 +66,7 @@ int virAllocN(void *ptrptr, size_t size, size_t count) { -*(void**)ptrptr = calloc(count, size); -if (*(void**)ptrptr == NULL) -abort(); - +*(void**)ptrptr = g_malloc0_n(count, size); return 0; } @@ -94,16 +88,7 @@ int virReallocN(void *ptrptr, size_t size, size_t count) { -void *tmp; - -if (xalloc_oversized(count, size)) -abort(); - -tmp = realloc(*(void**)ptrptr, size * count); -if (!tmp && ((size * count) != 0)) -abort(); - -*(void**)ptrptr = tmp; +*(void **)ptr