On Fri, May 30, 2008 at 03:37:06PM +0100, Daniel P. Berrange wrote:
THis patch switches over the remote daemon to use the memory
allocation
APIs. This required exporting the 4 apis in the .so. I discovered along
the way that none of the remote RPC dispatcher impls checked for malloc
failure, so I've had to add that in too.
Looks fine, this really cleans things up, especially reallocs
if (getsockname(sock->fd, (struct sockaddr *)(&sa),
&salen) < 0)
- return -1;
+ goto cleanup;
Hum, changes the semantic but it looks like it will avoid leaking
file descriptors too..
[..]
+
+cleanup:
+ for (i = 0; i < nfds; ++i)
+ close(fds[0]);
+ return -1;
[...]
- if (ret->freeMems.freeMems_len == 0)
+ if (ret->freeMems.freeMems_len == 0) {
+ VIR_FREE(ret->freeMems.freeMems_val);
return -1;
+ }
and memleaks ...
diff -r 06acc2c5c1fb src/memory.c
--- a/src/memory.c Thu May 29 16:05:55 2008 -0400
+++ b/src/memory.c Fri May 30 10:36:42 2008 -0400
@@ -104,7 +104,7 @@
*
* Returns -1 on failure to allocate, zero on success
*/
-int virAlloc(void *ptrptr, size_t size)
+int __virAlloc(void *ptrptr, size_t size)
{
#if TEST_OOM
if (virAllocTestFail()) {
@@ -137,7 +137,7 @@
*
* Returns -1 on failure to allocate, zero on success
*/
-int virAllocN(void *ptrptr, size_t size, size_t count)
+int __virAllocN(void *ptrptr, size_t size, size_t count)
{
#if TEST_OOM
if (virAllocTestFail()) {
@@ -171,7 +171,7 @@
*
* Returns -1 on failure to allocate, zero on success
*/
-int virReallocN(void *ptrptr, size_t size, size_t count)
+int __virReallocN(void *ptrptr, size_t size, size_t count)
{
void *tmp;
#if TEST_OOM
@@ -203,7 +203,7 @@
* the 'ptrptr' variable. After release, 'ptrptr' will be
* updated to point to NULL.
*/
-void virFree(void *ptrptr)
+void __virFree(void *ptrptr)
{
free(*(void**)ptrptr);
*(void**)ptrptr = NULL;
diff -r 06acc2c5c1fb src/memory.h
--- a/src/memory.h Thu May 29 16:05:55 2008 -0400
+++ b/src/memory.h Fri May 30 10:36:42 2008 -0400
@@ -26,10 +26,10 @@
#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 virReallocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK;
-void virFree(void *ptrptr);
+int __virAlloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK;
+int __virAllocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK;
+int __virReallocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK;
+void __virFree(void *ptrptr);
/**
* VIR_ALLOC:
@@ -41,7 +41,7 @@
*
* Returns -1 on failure, 0 on success
*/
-#define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr)))
+#define VIR_ALLOC(ptr) __virAlloc(&(ptr), sizeof(*(ptr)))
/**
* VIR_ALLOC_N:
@@ -54,7 +54,7 @@
*
* Returns -1 on failure, 0 on success
*/
-#define VIR_ALLOC_N(ptr, count) virAllocN(&(ptr), sizeof(*(ptr)), (count))
+#define VIR_ALLOC_N(ptr, count) __virAllocN(&(ptr), sizeof(*(ptr)), (count))
/**
* VIR_REALLOC_N:
@@ -67,7 +67,7 @@
*
* Returns -1 on failure, 0 on success
*/
-#define VIR_REALLOC_N(ptr, count) virReallocN(&(ptr), sizeof(*(ptr)), (count))
+#define VIR_REALLOC_N(ptr, count) __virReallocN(&(ptr), sizeof(*(ptr)), (count))
/**
* VIR_FREE:
@@ -76,7 +76,7 @@
* Free the memory stored in 'ptr' and update to point
* to NULL.
*/
-#define VIR_FREE(ptr) virFree(&(ptr));
+#define VIR_FREE(ptr) __virFree(&(ptr))
ah, right we need to export those symbols now ...
looks good to me, +1
Daniel
--
Red Hat Virtualization group
http://redhat.com/virtualization/
Daniel Veillard | virtualization library
http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit
http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine
http://rpmfind.net/