On 03/14/2011 09:33 AM, Eric Blake wrote: > On 03/14/2011 06:34 AM, Michal Privoznik wrote: >> in case of incorrect option parsing. >> --- >> My former patch introduced a regression: >> https://bugzilla.redhat.com/show_bug.cgi?id=605660 >> >> tools/virsh.c | 52 ++++++++++++++++++++++++++++++++++++++-------------- >> 1 files changed, 38 insertions(+), 14 deletions(-) >> >> diff --git a/tools/virsh.c b/tools/virsh.c >> index b42aac4..42ebd55 100644 >> --- a/tools/virsh.c >> +++ b/tools/virsh.c >> @@ -706,8 +706,10 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) >> } >> >> VIR_FREE(ctl->name); >> - if (vshCommandOptString(cmd, "name", &name) <= 0) >> + if (vshCommandOptString(cmd, "name", &name) < 0) { Actually, all your other changes were where pre-patch had < 0, so you were already prepared to deal with an optional argument. But for cmdConnect, you changed <= 0 to < 0, you now go on to the rest of the method with name still NULL, which causes problems on the subsequent strdup(). But an empty string for the URI is valid (it means the same as a NULL URI for requesting the default connection). Yet vshCommandOptString rejects the empty string. I'm pushing your patch as-is, but we need another followup for commands that specifically want to handle the empty string (such as cmdConnect) in a manner other than just rejecting it. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list