On 07/12/2011 01:47 AM, Hu Tao wrote:
>>>
>>> + ignore_value(vshCommandOptString(cmd, "cache", &cache));
>>
>> Not so nice.
>>
>> --cache ''
>>
>> will make vshCommandOptString return -1, because that usage is a virsh
>> usage error and should be diagnosed as such up front, rather than
>> accidentally passing cache='' through the XML to the libvirt API.
>
> I found that in the case of --cache '' vshCommandOptString returns 0,
> with cache(3rd parameter) unchanged. So can we safely ignore value or do
> I miss something?
vshCommandOptString currently returns:
1 if the option was provided and non-empty, or provided and empty and
VSH_OFLAG_EMPTY_OK
0 if the option was not provided, or provided but empty
-1 if the option was not provided but VSH_OFLAG_REQ
--cache isn't marked VSH_OFLAG_REQ, so we won't get -1. And we didn't
pass VSH_OFLAG_EMPTY_OK, so an empty string returns 0 instead of 1,
leaving the variable NULL the same as if --cache had not been provided.
But your code would be the first instance of using ignore_value on
vshCommandOptString. I'm starting to think that's a bug in the
implementation, and that vshCommandOptString should instead return:
1 if the option was provided and non-empty, or provided and empty and
VSH_OFLAG_EMPTY_OK
0 if the option was not provided
-1 if the option was not provided but VSH_OFLAG_REQ, or provided and
empty and not VSH_OFLAG_EMPTY_OK
in which case, it _does_ become important to check for errors.
> @@ -10209,6 +10254,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
> goto cleanup;
> }
>
> + ignore_value(vshCommandOptString(cmd, "cache", &cache));
> + ignore_value(vshCommandOptString(cmd, "serial", &serial));
> + ignore_value(vshCommandOptString(cmd, "pciaddress", &strpciaddr));
At any rate, we already have lots of existing code that looks like:
if (vshCommandOptString(cmd, "cache", &cache) < 0 ||
vshCommandOptString(cmd, "serial", &serial) < 0 ||
vshCommandOptString(cmd, "pciaddress", &strpciaddr) < 0) {
vshError(ctl, "%s", _("missing argument"));
goto out;
}
--
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