[Libvir] Virtual networking

[Karel Zak]

On Mon, Jan 22, 2007 at 02:46:11PM +0000, Mark McLoughlin wrote:
> # Add net-* commands to virsh
> #
> http://www.gnome.org/~markmc/code/libvirt-networking/libvirt-network-virsh.patch

 (IMHO it's better send a patch to mailing list than an URL...:-)


+    {"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("file containing an XML network description")},

 Cannot we use something like _N() rather than gettext_noop() ?


+    char buffer[4096];

 Don't use magic numbers. Use BUFSIZ.


+static vshCmdInfo info_network_list[] = {
+    {"syntax", "net-list"},

 {"syntax", "net-list [--inactive | --all]"},



+        names = vshMalloc(ctl, sizeof(char *) * maxname);
+
+        if ((maxname = virConnectListDefinedNetworks(ctl->conn, names, maxname)) < 0) {

 I'm not sure if I read livirt correctly, but where in virsh we deallocate name
strings?  It's not specific to your network patches. I see everywhere for
virConnectList* that we deallocate the "names" which is array of pointers only.

 See virsh.c: cmdList():

        names = vshMalloc(ctl, sizeof(char *) * maxname);

        if ((maxname = virConnectListDefinedDomains(ctl->conn, names, maxname)) < 0) {
     ....

        if (names)
          free(names);
        return TRUE;

 But in xend_internal.c: xenDaemonListDefinedDomains():

    names[ret++] = strdup(node->value);
                   ^^^^^^^
               where is free() for this string?


 It seems like nice leak(s). Right?



 +    {"syntax", "start a network "},

     {"syntax", "net-start <name>"},


+    char uuid[37];

  Magic number? :-)

  #define UUID_STRLEN	36

   char uuid[UUID_STRLEN+1];

  

 	Karel


-- 
 Karel Zak  <kzak at redhat.com>