Hi Karel,
Thanks for the review ...
Note, a lot of the code in the networking patches is just copied and
pasted from elsewhere in libvirt, so I'll fix up the original code
first.
On Tue, 2007-01-23 at 11:20 +0100, Karel Zak wrote:
> + {"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() ?
That's one for Dan ... I suspect it's just because gettext_noop()
worked with xgettext, whereas _N() didn't. We'd need to pass
--keyword=_N to xgettext.
(Also, it's always been N_() anywhere I've seen it)
> names[ret++] = strdup(node->value);
> ^^^^^^^
> where is free() for this string?
>
>
> It seems like nice leak(s). Right?
Yep, well spotted.
I've appended the patch I committed.
Thanks,
Mark.
Index: ChangeLog
===================================================================
RCS file: /data/cvs/libvirt/ChangeLog,v
retrieving revision 1.319
diff -u -p -r1.319 ChangeLog
--- ChangeLog 22 Jan 2007 20:43:02 -0000 1.319
+++ ChangeLog 23 Jan 2007 12:28:16 -0000
@@ -0,0 +1,7 @@
+Mon Jan 23 12:28:42 IST 2007 Mark McLoughlin <[EMAIL PROTECTED]>
+
+ Issues pointed out by Karel Zak <[EMAIL PROTECTED]>
+
+ * src/virsh.c: fix up some syntax strings, use BUFSIZ
+ and free names returned from virConnectListDefinedDomains()
+
Index: src/virsh.c
===================================================================
RCS file: /data/cvs/libvirt/src/virsh.c,v
retrieving revision 1.42
diff -u -p -r1.42 virsh.c
--- src/virsh.c 22 Jan 2007 20:43:02 -0000 1.42
+++ src/virsh.c 23 Jan 2007 12:28:16 -0000
@@ -309,7 +309,7 @@ cmdConnect(vshControl * ctl, vshCmd * cm
* "list" command
*/
static vshCmdInfo info_list[] = {
- {"syntax", "list"},
+ {"syntax", "list [--inactive | --all]"},
{"help", gettext_noop("list domains")},
{"desc", gettext_noop("Returns list of domains.")},
{NULL, NULL}
@@ -419,8 +419,10 @@ cmdList(vshControl * ctl, vshCmd * cmd A
virDomainPtr dom = virDomainLookupByName(ctl->conn, names[i]);
/* this kind of work with domains is not atomic operation */
- if (!dom)
+ if (!dom) {
+ free(names[i]);
continue;
+ }
ret = virDomainGetInfo(dom, &info);
id = virDomainGetID(dom);
@@ -439,6 +441,7 @@ cmdList(vshControl * ctl, vshCmd * cmd A
}
virDomainFree(dom);
+ free(names[i]);
}
if (ids)
free(ids);
@@ -546,7 +549,7 @@ cmdCreate(vshControl * ctl, vshCmd * cmd
char *from;
int found;
int ret = TRUE;
- char buffer[4096];
+ char buffer[BUFSIZ];
int fd, l;
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
@@ -601,7 +604,7 @@ cmdDefine(vshControl * ctl, vshCmd * cmd
char *from;
int found;
int ret = TRUE;
- char buffer[4096];
+ char buffer[BUFSIZ];
int fd, l;
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
@@ -677,7 +680,7 @@ cmdUndefine(vshControl * ctl, vshCmd * c
* "start" command
*/
static vshCmdInfo info_start[] = {
- {"syntax", "start a domain "},
+ {"syntax", "start <domain>"},
{"help", gettext_noop("start a (previously defined) inactive domain")},
{"desc", gettext_noop("Start a domain.")},
{NULL, NULL}
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
