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**)ptrptr
[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 **)ptrptr =
