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@xxxxxxxxxx>