From e85e34f3af7eec4cf1e2b326d77c3369cfb02e9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 7 Oct 2019 17:56:08 +0100 Subject: [PATCH] util: use glib memory allocation functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 = g_realloc_n(*(void**)ptrptr, size, count); return 0; } @@ -343,9 +328,7 @@ int virAllocVar(void *ptrptr, abort(); alloc_size = struct_size + (element_size * count); - *(void **)ptrptr = calloc(1, alloc_size); - if (*(void **)ptrptr == NULL) - abort(); + *(void **)ptrptr = g_malloc0(alloc_size); return 0; } @@ -362,7 +345,7 @@ void virFree(void *ptrptr) { int save_errno = errno; - free(*(void**)ptrptr); + g_free(*(void**)ptrptr); *(void**)ptrptr = NULL; errno = save_errno; } @@ -395,7 +378,7 @@ void virDispose(void *ptrptr, if (*(void**)ptrptr && count > 0) memset(*(void **)ptrptr, 0, count * element_size); - free(*(void**)ptrptr); + g_free(*(void**)ptrptr); *(void**)ptrptr = NULL; if (countptr) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 3e72e40bc9..517f9aada6 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -24,6 +24,15 @@ #include "internal.h" +/** + * DEPRECATION WARNING + * + * APIs in this file should only be used when modifying existing code. + * Consider converting existing code to use the new APIs when touching + * it. All new code must use the GLib memory allocation APIs and/or + * GLib array data types. See the hacking file for more guidance. + */ + /* Return 1 if an array of N objects, each of size S, cannot exist due to size arithmetic overflow. S must be positive and N must be nonnegative. This is a macro, not an inline function, so that it -- GitLab