[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: Switch all remaining code to memory alloc APIs



On Fri, May 30, 2008 at 04:01:39PM +0100, Daniel P. Berrange wrote:
> This patch switches all remaining code over to use the memory allocation
> APIs, with exception of virsh which is going to be slightly more complex
> 
> It was mostly a straight conversion - there were only a few places which 
> weren't checking for failure corecttly - the most notable being sexpr.c.
[...]
> -    void *stack, *stacktop;
> +    char *stack, *stacktop;
>  
>      /* allocate a stack for the container */
> -    stack = malloc(stacksize);
> -    if (!stack) {
> +    if (VIR_ALLOC_N(stack, stacksize) < 0) {

  hum, interesting side effect ... we must type stuff with the new macros.

[...]
> --- a/src/openvz_driver.c	Fri May 30 10:36:42 2008 -0400
> +++ b/src/openvz_driver.c	Fri May 30 10:55:44 2008 -0400
> @@ -49,8 +49,7 @@
>  #include <stdio.h>
>  #include <sys/wait.h>
>  
> -#include "libvirt/virterror.h"
> -
> +#include "internal.h"

  a bit of cleanup there too.

[...]
> -static void
> +static int
>  append(struct sexpr *lst, const struct sexpr *value)
>  {

  okay, the sexpr code really needed to be changed, good !

> diff -r ff6b92c70738 src/xen_internal.c
> --- a/src/xen_internal.c	Fri May 30 10:36:42 2008 -0400
> +++ b/src/xen_internal.c	Fri May 30 10:55:44 2008 -0400
...
> @@ -1659,8 +1659,7 @@
>          /* The allocated memory to cpumap must be 'sizeof(uint64_t)' byte *
>           * for Xen, and also nr_cpus must be 'sizeof(uint64_t) * 8'       */
>          if (maplen < 8) {
> -            new = calloc(1, sizeof(uint64_t));
> -            if (!new) {
> +            if (VIR_ALLOC_N(new, sizeof(uint64_t)) < 0) {

  That one worried me, but that works but because we have unsigned char *new

> --- a/src/xmlrpc.c	Fri May 30 10:36:42 2008 -0400
> +++ b/src/xmlrpc.c	Fri May 30 10:55:44 2008 -0400
> @@ -12,6 +12,7 @@

  Hum, i think that's dead code anyway, no ?

>  #include "xmlrpc.h"
>  #include "internal.h"
> +#include "memory.h"
>  
>  #include <libxml/nanohttp.h>
>  
> @@ -47,9 +48,8 @@
>  
>  static xmlRpcValuePtr xmlRpcValueNew(xmlRpcValueType type)
>  {
> -    xmlRpcValuePtr ret = malloc(sizeof(*ret));
> -
> -    if (!ret)
> +    xmlRpcValuePtr ret = NULL;
> +    if (VIR_ALLOC(ret) < 0)

  I don't think we need to set ret to NULL, do we ? VIR_ALLOC always
initialize.


  Okay I think I really read all the patch and tried to check everything :-)

    looks good, thanks a lot !

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]