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 <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/bootstrap.conf b/bootstrap.conf
index 358d783a6b044b0d64e69be0f4384a0b0457417a..7d73584809dde98b94484b509cfd9a1889c7b34b 100644 (file)
--- 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 2e064ced5e6a0a82622717a09cb517fbe34f6dea..807279631264ed5c5d694e073ac11b69bfd43662 100644 (file)
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1008,102 +1008,20 @@ BAD:
     </p>
 
     <dl>
+      <dt>VIR_ALLOC, VIR_REALLOC, VIR_RESIZE_N, VIR_EXPAND_N,
+        VIR_SHRINK_N, VIR_FREE, VIR_APPEND_ELEMENT, VIR_INSERT_ELEMENT,
+        VIR_DELETE_ELEMENT</dt>
+      <dd>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. <strong>NEVER MIX</strong> 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.</dd>
     </dl>
 
-    <h2><a id="memalloc">Low level memory management</a></h2>
-
-    <p>
-      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.
-    </p>
-
-    <ul>
-      <li><p>To allocate a single object:</p>
-
-<pre>
-  virDomainPtr domain;
-
-  if (VIR_ALLOC(domain) &lt; 0)
-      return NULL;
-</pre>
-      </li>
-
-      <li><p>To allocate an array of objects:</p>
-<pre>
-  virDomainPtr domains;
-  size_t ndomains = 10;
-
-  if (VIR_ALLOC_N(domains, ndomains) &lt; 0)
-      return NULL;
-</pre>
-      </li>
-
-      <li><p>To allocate an array of object pointers:</p>
-<pre>
-  virDomainPtr *domains;
-  size_t ndomains = 10;
-
-  if (VIR_ALLOC_N(domains, ndomains) &lt; 0)
-      return NULL;
-</pre>
-      </li>
-
-      <li><p>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):</p>
-<pre>
-  virDomainPtr domains;
-  size_t ndomains = 0;
-
-  if (VIR_EXPAND_N(domains, ndomains, 1) &lt; 0)
-      return NULL;
-  domains[ndomains - 1] = domain;
-</pre></li>
-
-      <li><p>To ensure an array has room to hold at least one more
-      element (this approach scales better, but requires tracking
-      allocation separately from usage)</p>
-
-<pre>
-  virDomainPtr domains;
-  size_t ndomains = 0;
-  size_t ndomains_max = 0;
-
-  if (VIR_RESIZE_N(domains, ndomains_max, ndomains, 1) &lt; 0)
-      return NULL;
-  domains[ndomains++] = domain;
-</pre>
-      </li>
-
-      <li><p>To trim an array of domains from its allocated size down
-      to the actual used size:</p>
-
-<pre>
-  virDomainPtr domains;
-  size_t ndomains = x;
-  size_t ndomains_max = y;
-
-  VIR_SHRINK_N(domains, ndomains_max, ndomains_max - ndomains);
-</pre></li>
-
-      <li><p>To free an array of domains:</p>
-<pre>
-  virDomainPtr domains;
-  size_t ndomains = x;
-  size_t ndomains_max = y;
-  size_t i;
-
-  for (i = 0; i &lt; ndomains; i++)
-      VIR_FREE(domains[i]);
-  VIR_FREE(domains);
-  ndomains_max = ndomains = 0;
-</pre>
-      </li>
-    </ul>
-
     <h2><a id="file_handling">File handling</a></h2>
 
     <p>

diff --git a/src/util/viralloc.c b/src/util/viralloc.c
index 10a8d0fb737852b74dfddfc652d5bb83b71f57fb..b8ca850764fe150a3c2e9a3e2a8eb00c444e254e 100644 (file)
--- 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 3e72e40bc93b643bf723bd0cb3c26d84b2478739..517f9aada6216555aaebeb49201f1c0a3a1ea200 100644 (file)
--- 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