After updating the virBuffer APIs to protect against improper usage I have
been thinking about how we might provider safer memory allocation APIs
with protection against common usage errors and compile time validation of
checks for failure.
The standard C library malloc/realloc/calloc/free APIs have just about the
worst possible design, positively encouraging serious application coding
errors. There are at least 7 common problems I can think of at the moment,
perhaps more....
1. malloc() - pass incorrect number of bytes
2. malloc() - fail to check the return value
3. malloc() - use uninitialized memory
4. free() - double free by not setting pointer to NULL
5. realloc() - fail to check the return value
6. realloc() - fail to re-assign the pointer with new address
7. realloc() - accidental overwrite of original pointer with NULL on
failure, leaking memory
Many applications / libraries wrap these in their own functions, but typically
fail to address one or more these problems.
eg glib re-definitions try to address point 2 by having g_malloc() call
abort() on failure, but then re-introduce the problem by adding g_try_malloc()
gpointer g_malloc (gulong n_bytes) G_GNUC_MALLOC;
gpointer g_try_malloc (gulong n_bytes) G_GNUC_MALLOC;
Simiarly it tries address point 6, with the annotation, but this still leaves
open the failure to check for NULL in point 1.
gpointer g_realloc (gpointer mem,
gulong n_bytes) G_GNUC_WARN_UNUSED_RESULT;
gpointer g_try_realloc (gpointer mem,
gulong n_bytes) G_GNUC_WARN_UNUSED_RESULT;
And the g_free wrapper doesn't help with the double free issue at all:
void g_free (gpointer mem);
All 7 of the errors listed early could be avoided and/or checked at compile
time if the APIs used a different calling convention.
1. For any given pointer the compiler knows how many bytes need to be
allocated for a single instance of it. ie sizeof(*(ptr)). Since C
has no data type representing a data type, this cannot be done with
a simple function - it needs a (trivial) macro.
2. The __attribute__((__warn_unused_result__)) annotation causes the
compiler to warn if an application doesn't do something with a return
value. With the standard malloc() API this doesn't help because you
always assign the return value to a pointer. The core problem here
is that the API encodes the error condition as a special case of
the actual data returned. This can be addressed by separating the
two. The return value should simply be bi-state success or fail code
and the return data passed back via an pointer to a pointer parameter.
3. The memory allocated by malloc() is not initialized to zero. For
that a separate calloc() function is provided. This leaves open the
possiblity of an app mistakenly using the wrong variant. Or consider
an app using malloc() and explicitly initializing all struct members.
Some time later a new member is added and now the developer needs to
find all malloc() calls wrt to that struct and explicitly initilize
the new member. It would be safer to always have malloc zero all
memory, even though it has a small performance impact, the net win
in safety is probably worth it.
4. Since free() takes the pointer to be freed directly it cannot reset
the caller's original pointer back to NULL. This can be addressed if
a pointer to the pointer is passed instead. The computational cost
of the often redundant assignment to NULL is negligable given the
safety benefit
5, 6 & 7. As with problem 2, these problems are all caused by the fact
that the error condition is special case of the return data value.
The impact of these is typically far worse because it often results
in a buffer overflow and the complex rules for calling realloc() are
hard to remember.
So the core requirements of a safer API for allocation are:
- Use return values *only* for the success/fail error condition
- Annotate all the return values with __warn_unused_result__
- Pass a pointer to the pointer into all functions
- Define macros around all functions to automatically do the sizeof(*(ptr))
Passing a pointer to a pointer gets a little tedious because of the need
to write '&(foo)' instead of just 'foo', and is actually introduces a
new problem of the caller accidentally passing merely a pointer, instead
of a pointer to a pointer. This is a minor problem though because it will
be identified on the very first run of the code. In addition, since we're
defining macros around every function we can have the macro also convert
from ptr to &(ptr) for us, avoiding this potential error:
So the primary application usage would be via a set of macros:
VIR_ALLOC(ptr)
VIR_ALLOC_N(ptr, count)
VIR_REALLOC(ptr)
VIR_REALLOC_N(ptr, count)
VIR_FREE(ptr)
These call through to the underlying APIs:
int virAlloc(void *, size_t bytes)
int virAllocN(void *, size_t bytes, size_t count)
int virRealloc(void *, size_t bytes)
int virReallocN(void *, size_t bytes, size_t count)
int virFree(void *)
Note that although we're passing a pointer to a pointer into all these, the
first param is still just 'void *' and not 'void **'. This is because 'void *'
is defined to hold any type of pointer, and using 'void **' causes gcc to
complain bitterly about strict aliasing violations. Internally the impls of
these methods will safely cast to 'void **' when deferencing the pointer.
All 4 of Alloc/Realloc functions will have __warn_unused_result__ annotation
so the caller is forced to check the return value for failure, validated at
compile time generating a warning (or fatal compile error with -Werror).
So to wire up the macros to the APIs:
#define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr)))
#define VIR_ALLOC_N(ptr, count) virAllocN(&(ptr), sizeof(*(ptr)), (count))
#define VIR_REALLOC(ptr) virRealloc(&(ptr), sizeof(*(ptr)))
#define VIR_REALLOC_N(ptr, count) virReallocN(&(ptr), sizeof(*(ptr)), (count))
#define VIR_FREE(ptr) virFree(&(ptr))
In particular notice how we're using the compiler to decide how many bytes
to allocate for each variable here, thus guarenteeing the correct size.
Finally here's an illustration of how these APis would be used in practice.
- Allocating a new variable:
virDomainPtr domain;
if (VIR_ALLOC(domain) < 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
Looking at the macro, this pass '&domain' into virAlloc() which initalizes
it with address of memory sizeof(*domain) bytes long.
- Allocating an array of domains
virDomainPtr domains;
int ndomains = 10;
if (VIR_ALLOC_N(domains, ndomains) < 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
- Allocating an array of domain pointers
virDomainPtr *domains;
int ndomains = 10;
if (VIR_ALLOC_N(domains, ndomains) < 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
- Re-allocate the array of domains to be longer
ndomains = 20
if (VIR_REALLOC_N(domains, ndomains) < 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
- Free the domain
VIR_FREE(domain);
The attached patch implements these ideas in a memory.h and memory.c file
and updates 'hash.c' to illustrate their usage. If we were to replace
every single instance of malloc/calloc/free/realloc with these calls the
automated builds which run with -Werror would quickly tell us of any places
where we forget to check memory allocations for failure, and we'd make
code like from capabilities.c:
if ((guest->arch.defaultInfo.machines =
calloc(nmachines,
sizeof(*guest->arch.defaultInfo.machines))) == NULL)
return -1;
char **migrateTrans;
if ((migrateTrans = realloc(caps->host.migrateTrans,
sizeof(*migrateTrans) *
(caps->host.nmigrateTrans+1))) == NULL)
return -1;
caps->host.migrateTrans = migrateTrans;
Much less ugly:
if (VIR_ALLOC_N(guest->arch.defaultInfo.machines, nmachines) < 0)
return -1;
if (VIR_REALLOC(migrateTrans, caps->host.nmigrateTrans+1) < 0)
return -1;
So we've addressed all 7 common error scenarios with malloc/free/realloc/caller
and produced easier to read code too. The cleaner realloc() API is particularly
appealing.
This does all seem a little bit too good to be true - I'm wondering why other
apps/libraries don't use this style of allocation functions already ? Perhaps
someone can find nasty flaws in this approach, but hopefuly not...
hash.c | 113 ++++++++++++++++++++++--------------------------
memory.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
memory.h | 95 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 294 insertions(+), 60 deletions(-)
Regards,
Daniel.
Index: memory.h
===================================================================
RCS file: memory.h
diff -N memory.h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ memory.h 27 Apr 2008 19:04:28 -0000
@@ -0,0 +1,95 @@
+/*
+ * memory.c: safer memory allocation
+ *
+ * Copyright (C) 2008 Daniel P. Berrange
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+
+#ifndef __VIR_MEMORY_H_
+#define __VIR_MEMORY_H_
+
+#include "internal.h"
+
+/* 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 virRealloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK;
+int virReallocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK;
+void virFree(void *ptrptr);
+
+
+/**
+ * VIR_ALLOC:
+ * @ptr: pointer to hold address of allocated memory
+ *
+ * Allocate sizeof(*ptr) bytes of memory and store
+ * the address of allocated memory in 'ptr'. Fill the
+ * newly allocated memory with zeros.
+ *
+ * Returns -1 on failure, 0 on success
+ */
+#define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr)))
+
+/**
+ * VIR_ALLOC_N:
+ * @ptr: pointer to hold address of allocated memory
+ * @count: number of elements to allocate
+ *
+ * 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.
+ *
+ * Returns -1 on failure, 0 on success
+ */
+#define VIR_ALLOC_N(ptr, count) virAllocN(&(ptr), sizeof(*(ptr)), (count))
+
+/**
+ * VIR_REALLOC:
+ * @ptr: pointer to hold address of allocated memory
+ *
+ * Re-allocate to sizeof(*ptr) bytes of memory and store
+ * the address of allocated memory in 'ptr'. Fill the
+ * newly allocated memory with zeros
+ *
+ * Returns -1 on failure, 0 on success
+ */
+#define VIR_REALLOC(ptr) virRealloc(&(ptr), sizeof(*(ptr)))
+
+/**
+ * VIR_REALLOC_N:
+ * @ptr: pointer to hold address of allocated memory
+ * @count: number of elements to allocate
+ *
+ * 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
+ *
+ * Returns -1 on failure, 0 on success
+ */
+#define VIR_REALLOC_N(ptr, count) virReallocN(&(ptr), sizeof(*(ptr)), (count))
+
+/**
+ * VIR_FREE:
+ * @ptr: pointer holding address to be freed
+ *
+ * Free the memory stored in 'ptr' and update to point
+ * to NULL.
+ */
+#define VIR_FREE(ptr) virFree(&(ptr))
+
+#endif /* __VIR_MEMORY_H_ */
Index: memory.c
===================================================================
RCS file: memory.c
diff -N memory.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ memory.c 27 Apr 2008 19:04:28 -0000
@@ -0,0 +1,146 @@
+/*
+ * memory.c: safer memory allocation
+ *
+ * Copyright (C) 2008 Daniel P. Berrange
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include <stdlib.h>
+
+#include "memory.h"
+
+/**
+ * virAlloc:
+ * @ptrptr: pointer to pointer for address of allocated memory
+ * @size: number of bytes to allocate
+ *
+ * Allocate 'size' bytes of memory. Return the address of the
+ * allocated memory in 'ptrptr'. The newly allocated memory is
+ * filled with zeros.
+ *
+ * Returns -1 on failure to allocate, zero on success
+ */
+int virAlloc(void *ptrptr, size_t size)
+{
+ if (size == 0) {
+ *(void **)ptrptr = NULL;
+ return 0;
+ }
+
+ *(void **)ptrptr = calloc(1, size);
+ if (*(void **)ptrptr == NULL)
+ return -1;
+ return 0;
+}
+
+/**
+ * virAllocN:
+ * @ptrptr: pointer to pointer for address of allocated memory
+ * @size: number of bytes to allocate
+ * @count: number of elements to allocate
+ *
+ * Allocate an array of memory 'count' elements long,
+ * each with 'size' bytes. Return the address of the
+ * allocated memory in 'ptrptr'. The newly allocated
+ * memory is filled with zeros.
+ *
+ * Returns -1 on failure to allocate, zero on success
+ */
+int virAllocN(void *ptrptr, size_t size, size_t count)
+{
+ if (size == 0 || count == 0) {
+ *(void **)ptrptr = NULL;
+ return 0;
+ }
+
+ *(void**)ptrptr = calloc(count, size);
+ if (*(void**)ptrptr == NULL)
+ return -1;
+ return 0;
+}
+
+/**
+ * virRealloc:
+ * @ptrptr: pointer to pointer for address of allocated memory
+ * @size: number of bytes to allocate
+ *
+ * Resize the block of memory in 'ptrptr' to be '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.
+ *
+ * Returns -1 on failure to allocate, zero on success
+ */
+int virRealloc(void *ptrptr, size_t size)
+{
+ void *tmp;
+ if (size == 0) {
+ free(*(void **)ptrptr);
+ *(void **)ptrptr = NULL;
+ return 0;
+ }
+
+ tmp = realloc(*(void**)ptrptr, size);
+ if (!tmp)
+ return -1;
+ *(void**)ptrptr = tmp;
+ return 0;
+}
+
+/**
+ * virReallocN:
+ * @ptrptr: pointer to pointer for address of allocated memory
+ * @size: number of bytes to allocate
+ * @count: number of elements in array
+ *
+ * Resize the block of memory in 'ptrptr' to be an array of
+ * '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.
+ *
+ * Returns -1 on failure to allocate, zero on success
+ */
+int virReallocN(void *ptrptr, size_t size, size_t count)
+{
+ void *tmp;
+ if (size == 0 || count == 0) {
+ free(*(void **)ptrptr);
+ *(void **)ptrptr = NULL;
+ return 0;
+ }
+ tmp = realloc(*(void**)ptrptr, size * count);
+ if (!tmp)
+ return -1;
+ *(void**)ptrptr = tmp;
+ return 0;
+}
+
+/**
+ * virFree:
+ * @ptrptr: pointer to pointer for address of memory to be freed
+ *
+ * Release the chunk of memory in the pointer pointed to by
+ * the 'ptrptr' variable. After release, 'ptrptr' will be
+ * updated to point to NULL.
+ */
+void virFree(void *ptrptr)
+{
+ free(*(void**)ptrptr);
+ *(void**)ptrptr = NULL;
+}
Index: hash.c
===================================================================
RCS file: /data/cvs/libvirt/src/hash.c,v
retrieving revision 1.37
diff -u -p -r1.37 hash.c
--- hash.c 18 Apr 2008 08:33:23 -0000 1.37
+++ hash.c 27 Apr 2008 19:04:28 -0000
@@ -25,6 +25,7 @@
#include <libxml/threads.h>
#include "internal.h"
#include "hash.h"
+#include "memory.h"
#define MAX_HASH_LEN 8
@@ -85,22 +86,22 @@ virHashComputeKey(virHashTablePtr table,
virHashTablePtr
virHashCreate(int size)
{
- virHashTablePtr table;
+ virHashTablePtr table = NULL;
if (size <= 0)
size = 256;
- table = malloc(sizeof(*table));
- if (table) {
- table->size = size;
- table->nbElems = 0;
- table->table = calloc(1, size * sizeof(*(table->table)));
- if (table->table) {
- return (table);
- }
- free(table);
+ if (VIR_ALLOC(table) < 0)
+ return NULL;
+
+ table->size = size;
+ table->nbElems = 0;
+ if (VIR_ALLOC_N(table->table, size) < 0) {
+ VIR_FREE(table);
+ return NULL;
}
- return (NULL);
+
+ return table;
}
/**
@@ -136,8 +137,7 @@ virHashGrow(virHashTablePtr table, int s
if (oldtable == NULL)
return (-1);
- table->table = calloc(1, size * sizeof(*(table->table)));
- if (table->table == NULL) {
+ if (VIR_ALLOC_N(table->table, size) < 0) {
table->table = oldtable;
return (-1);
}
@@ -170,7 +170,7 @@ virHashGrow(virHashTablePtr table, int s
if (table->table[key].valid == 0) {
memcpy(&(table->table[key]), iter, sizeof(virHashEntry));
table->table[key].next = NULL;
- free(iter);
+ VIR_FREE(iter);
} else {
iter->next = table->table[key].next;
table->table[key].next = iter;
@@ -184,7 +184,7 @@ virHashGrow(virHashTablePtr table, int s
}
}
- free(oldtable);
+ VIR_FREE(oldtable);
#ifdef DEBUG_GROW
xmlGenericError(xmlGenericErrorContext,
@@ -225,19 +225,19 @@ virHashFree(virHashTablePtr table, virHa
next = iter->next;
if ((f != NULL) && (iter->payload != NULL))
f(iter->payload, iter->name);
- free(iter->name);
+ VIR_FREE(iter->name);
iter->payload = NULL;
if (!inside_table)
- free(iter);
+ VIR_FREE(iter);
nbElems--;
inside_table = 0;
iter = next;
}
inside_table = 0;
}
- free(table->table);
+ VIR_FREE(table->table);
}
- free(table);
+ VIR_FREE(table);
}
/**
@@ -281,8 +281,7 @@ virHashAddEntry(virHashTablePtr table, c
if (insert == NULL) {
entry = &(table->table[key]);
} else {
- entry = malloc(sizeof(*entry));
- if (entry == NULL)
+ if (VIR_ALLOC(entry) < 0)
return (-1);
}
@@ -354,8 +353,7 @@ virHashUpdateEntry(virHashTablePtr table
if (insert == NULL) {
entry = &(table->table[key]);
} else {
- entry = malloc(sizeof(*entry));
- if (entry == NULL)
+ if (VIR_ALLOC(entry) < 0)
return (-1);
}
@@ -451,10 +449,10 @@ virHashRemoveEntry(virHashTablePtr table
if ((f != NULL) && (entry->payload != NULL))
f(entry->payload, entry->name);
entry->payload = NULL;
- free(entry->name);
+ VIR_FREE(entry->name);
if (prev) {
prev->next = entry->next;
- free(entry);
+ VIR_FREE(entry);
} else {
if (entry->next == NULL) {
entry->valid = 0;
@@ -462,7 +460,7 @@ virHashRemoveEntry(virHashTablePtr table
entry = entry->next;
memcpy(&(table->table[key]), entry,
sizeof(virHashEntry));
- free(entry);
+ VIR_FREE(entry);
}
}
table->nbElems--;
@@ -535,11 +533,11 @@ int virHashRemoveSet(virHashTablePtr tab
if (iter(entry->payload, entry->name, data)) {
count++;
f(entry->payload, entry->name);
- free(entry->name);
+ VIR_FREE(entry->name);
table->nbElems--;
if (prev) {
prev->next = entry->next;
- free(entry);
+ VIR_FREE(entry);
entry = prev;
} else {
if (entry->next == NULL) {
@@ -549,7 +547,7 @@ int virHashRemoveSet(virHashTablePtr tab
entry = entry->next;
memcpy(&(table->table[i]), entry,
sizeof(virHashEntry));
- free(entry);
+ VIR_FREE(entry);
entry = &(table->table[i]);
continue;
}
@@ -689,8 +687,7 @@ virConnectPtr
virGetConnect(void) {
virConnectPtr ret;
- ret = calloc(1, sizeof(*ret));
- if (ret == NULL) {
+ if (VIR_ALLOC(ret) < 0) {
virHashError(NULL, VIR_ERR_NO_MEMORY, _("allocating connection"));
goto failed;
}
@@ -729,7 +726,7 @@ failed:
virHashFree(ret->storageVols, (virHashDeallocator) virStorageVolFreeName);
pthread_mutex_destroy(&ret->lock);
- free(ret);
+ VIR_FREE(ret);
}
return(NULL);
}
@@ -759,11 +756,11 @@ virReleaseConnect(virConnectPtr conn) {
if (__lastErr.conn == conn)
__lastErr.conn = NULL;
- free(conn->name);
+ VIR_FREE(conn->name);
pthread_mutex_unlock(&conn->lock);
pthread_mutex_destroy(&conn->lock);
- free(conn);
+ VIR_FREE(conn);
}
/**
@@ -824,8 +821,7 @@ __virGetDomain(virConnectPtr conn, const
ret = (virDomainPtr) virHashLookup(conn->domains, name);
/* TODO check the UUID */
if (ret == NULL) {
- ret = (virDomainPtr) calloc(1, sizeof(*ret));
- if (ret == NULL) {
+ if (VIR_ALLOC(ret) < 0) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating domain"));
goto error;
}
@@ -854,8 +850,8 @@ __virGetDomain(virConnectPtr conn, const
error:
pthread_mutex_unlock(&conn->lock);
if (ret != NULL) {
- free(ret->name );
- free(ret);
+ VIR_FREE(ret->name);
+ VIR_FREE(ret);
}
return(NULL);
}
@@ -888,8 +884,8 @@ virReleaseDomain(virDomainPtr domain) {
__lastErr.dom = NULL;
domain->magic = -1;
domain->id = -1;
- free(domain->name);
- free(domain);
+ VIR_FREE(domain->name);
+ VIR_FREE(domain);
DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs);
conn->refs--;
@@ -962,8 +958,7 @@ __virGetNetwork(virConnectPtr conn, cons
ret = (virNetworkPtr) virHashLookup(conn->networks, name);
/* TODO check the UUID */
if (ret == NULL) {
- ret = (virNetworkPtr) calloc(1, sizeof(*ret));
- if (ret == NULL) {
+ if (VIR_ALLOC(ret) < 0) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating network"));
goto error;
}
@@ -991,8 +986,8 @@ __virGetNetwork(virConnectPtr conn, cons
error:
pthread_mutex_unlock(&conn->lock);
if (ret != NULL) {
- free(ret->name );
- free(ret);
+ VIR_FREE(ret->name);
+ VIR_FREE(ret);
}
return(NULL);
}
@@ -1025,8 +1020,8 @@ virReleaseNetwork(virNetworkPtr network)
__lastErr.net = NULL;
network->magic = -1;
- free(network->name);
- free(network);
+ VIR_FREE(network->name);
+ VIR_FREE(network);
DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs);
conn->refs--;
@@ -1100,8 +1095,7 @@ __virGetStoragePool(virConnectPtr conn,
ret = (virStoragePoolPtr) virHashLookup(conn->storagePools, name);
/* TODO check the UUID */
if (ret == NULL) {
- ret = (virStoragePoolPtr) calloc(1, sizeof(*ret));
- if (ret == NULL) {
+ if (VIR_ALLOC(ret) < 0) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating storage pool"));
goto error;
}
@@ -1129,8 +1123,8 @@ __virGetStoragePool(virConnectPtr conn,
error:
pthread_mutex_unlock(&conn->lock);
if (ret != NULL) {
- free(ret->name);
- free(ret);
+ VIR_FREE(ret->name);
+ VIR_FREE(ret);
}
return(NULL);
}
@@ -1159,8 +1153,8 @@ virReleaseStoragePool(virStoragePoolPtr
_("pool missing from connection hash table"));
pool->magic = -1;
- free(pool->name);
- free(pool);
+ VIR_FREE(pool->name);
+ VIR_FREE(pool);
DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs);
conn->refs--;
@@ -1232,8 +1226,7 @@ __virGetStorageVol(virConnectPtr conn, c
ret = (virStorageVolPtr) virHashLookup(conn->storageVols, key);
if (ret == NULL) {
- ret = (virStorageVolPtr) calloc(1, sizeof(*ret));
- if (ret == NULL) {
+ if (VIR_ALLOC(ret) < 0) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating storage vol"));
goto error;
}
@@ -1266,9 +1259,9 @@ __virGetStorageVol(virConnectPtr conn, c
error:
pthread_mutex_unlock(&conn->lock);
if (ret != NULL) {
- free(ret->name);
- free(ret->pool);
- free(ret);
+ VIR_FREE(ret->name);
+ VIR_FREE(ret->pool);
+ VIR_FREE(ret);
}
return(NULL);
}
@@ -1297,9 +1290,9 @@ virReleaseStorageVol(virStorageVolPtr vo
_("vol missing from connection hash table"));
vol->magic = -1;
- free(vol->name);
- free(vol->pool);
- free(vol);
+ VIR_FREE(vol->name);
+ VIR_FREE(vol->pool);
+ VIR_FREE(vol);
DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs);
conn->refs--;
--
|: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.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