On 01/17/2013 08:51 AM, Benoît Canet wrote: > Valid quorum resources look like > quorum:threshold/total:path/to/image_1: ... :path/to/image_total > > ':' is used as a separator > '\' is the escaping character for filename containing ':' > '\' escape itself > ',' must be escaped with ',' > > On the command line for quorum files "img:test.raw", "img2,raw" > and "img3.raw" invocation look like: > > -drive file=quorum:2/3:img\\:test.raw:img2,,raw:img3.raw > (note the double \\ and the double ,,) > > Signed-off-by: Benoit Canet <ben...@irqsave.net>
> + /* Get threshold */
> + errno = 0;
> + s->threshold = strtoul(start, &a, 10);
> + if (*a != '/' || errno) {
> + return -EINVAL;
> + }
> + a++;
Hmm - you can fail to reject file=quorum:/3:... (strtoul happily parses
a to 0 in that case, and is not required to set errno). But see below...
> +
> + /* Get total */
> + errno = 0;
> + s->total = strtoul(a, &b, 10);
> + if (*b != ':' || errno) {
> + return -EINVAL;
> + }
> + b++;
Again, you fail to reject file=qourum:1/:... (strtoul happily parses b
to 0 in that case, and is not required to set errno)...
> +
> + if (s->threshold < 1 || s->total < 2) {
> + return -EINVAL;
> + }
...but you got lucky: this check rejects either a or b being set to 0.
Still, you may want to refactor this patch on top of
https://lists.gnu.org/archive/html/qemu-devel/2013-01/msg03238.html.
> + if ((j + 1) != s->total) {
> + ret = -EINVAL;
> + goto free_exit;
> + }
You have a lot of reasons why this function can fail with -EINVAL; it
would be nicer if you actually set an error object describing each
failure, instead of making the user guess.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
