[libvirt] PATCH: Switch remote daemon to use memory alloc APIs

[Daniel Veillard]

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 at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/