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

27 Jul 2009


      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/

    