Re: [libvirt] [PATCH v2] add --cache, --serial, --sharable and --pciaddress options(was Re: [PATCH 1/3] add --cache option for attach-disk)

On 07/12/2011 01:47 AM, Hu Tao wrote: 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. 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(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org