[libvirt] [PATCH] virsh: Insert error messages to avoid a quiet abortion of commands

Eric Blake eblake at redhat.com
Mon Mar 14 16:09:41 UTC 2011


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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110314/280765b8/attachment-0001.sig>


More information about the libvir-list mailing list