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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list