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

Re: [libvirt] [PATCH 3/6] Add virCapsGuestMachine structure



On Thu, Jul 23, 2009 at 06:34:41PM +0100, Mark McLoughlin wrote:
> A subsequent commit will add a "canonical" field to this structure,
> this patch basically just prepares the way for that.
> 
> The new type is added, along with virCapabilitiesAlloc/FreeMachines()
> helpers and a whole bunch of code to make the transition.
> 
> One quirk is that virCapabilitiesAddGuestDomain() and
> virCapabilitiesAddGuest() take ownership of the machine list rather
> than duping it. This makes sense to avoid needless copying.
> 
> * src/capabilities.h: add the virCapsGuestMachine struct and use it
>   in virCapsGuestDomainInfo, add prototypes for new functions and
>   update the AddGuest() prototypes
> 
> * src/capabilities.c: add code for allocating and freeing the new
>   type, change the machines parameter to AddGuest() etc.
> 
> * src/libvirt_private.syms: export the new helpers
> 
> * src/qemu_conf.c: update all the machine type code to use the new
>   struct
> 
> * src/xen_internal.c: ditto
> 
> * tests/testutilsqemu.c: ditto
[...]
> +virCapsGuestMachinePtr *
> +virCapabilitiesAllocMachines(const char *const *names, int nnames)
> +{
> +    virCapsGuestMachinePtr *machines;
> +    int i;
> +
> +    if (VIR_ALLOC_N(machines, nnames) < 0)
> +        return NULL;
> +
> +    for (i = 0; i < nnames; i++) {
> +        if (VIR_ALLOC(machines[i]) < 0 ||
> +            !(machines[i]->name = strdup(names[i]))) {

  we should emit an OOM error here

> @@ -296,10 +296,16 @@ qemudParseMachineTypesStr(const char *output,
>          if (!(t = strchr(p, ' ')) || (next && t >= next))
>              continue;
>  
> -        if (!(machine = strndup(p, t - p)))
> +        if (VIR_ALLOC(machine) < 0)
>              goto error;
>  
> +        if (!(machine->name = strndup(p, t - p))) {
> +            VIR_FREE(machine);
> +            goto error;
> +        }
> +
>          if (VIR_REALLOC_N(list, nitems + 1) < 0) {
> +            VIR_FREE(machine->name);
>              VIR_FREE(machine);
>              goto error;
>          }

  Same here the strndup failure paths should emit OOM errors
> @@ -321,15 +327,13 @@ qemudParseMachineTypesStr(const char *output,
>      return 0;
>  
>  error:
> -    for (i = 0; i < nitems; i++)
> -        VIR_FREE(list[i]);
> -    VIR_FREE(list);
> +    virCapabilitiesFreeMachines(list, nitems);
>      return -1;
>  }

  maybe a no_memory:  label with the call would be the simplest,
we use that in other places.

> @@ -434,12 +438,18 @@ qemudCapsInitGuest(virCapsPtr caps,
>          return 0;
>  
>      if (info->machine) {
> -        char *machine;
> +        virCapsGuestMachinePtr machine;
> +
> +        if (VIR_ALLOC(machine) < 0)
> +            return -1;
>  
> -        if (!(machine = strdup(info->machine)))
> +        if (!(machine->name = strdup(info->machine))) {
> +            VIR_FREE(machine);
>              return -1;
> +        }
>  
>          if (VIR_ALLOC_N(machines, nmachines) < 0) {
> +            VIR_FREE(machine->name);
>              VIR_FREE(machine);
>              return -1;
>          }

  similar

  ACK, but it would be better if those where cleaned up before commit.

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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