From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33025) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tvxhy-0004xf-Pt for qemu-devel@nongnu.org; Thu, 17 Jan 2013 17:13:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tvxhw-0001tU-U6 for qemu-devel@nongnu.org; Thu, 17 Jan 2013 17:13:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:6531) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tvxhw-0001tJ-MS for qemu-devel@nongnu.org; Thu, 17 Jan 2013 17:13:12 -0500 Message-ID: <50F87770.2030809@redhat.com> Date: Thu, 17 Jan 2013 15:13:04 -0700 From: Eric Blake MIME-Version: 1.0 References: <1358437897-24251-1-git-send-email-benoit@irqsave.net> <1358437897-24251-4-git-send-email-benoit@irqsave.net> In-Reply-To: <1358437897-24251-4-git-send-email-benoit@irqsave.net> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2OILSQECHKEPIAVQMQQIC" Subject: Re: [Qemu-devel] [RFC V6 03/11] quorum: Add quorum_open() and quorum_close(). List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2OILSQECHKEPIAVQMQQIC Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 01/17/2013 08:51 AM, Beno=C3=AEt Canet wrote: > Valid quorum resources look like > quorum:threshold/total:path/to/image_1: ... :path/to/image_total >=20 > ':' is used as a separator > '\' is the escaping character for filename containing ':' > '\' escape itself > ',' must be escaped with ',' >=20 > On the command line for quorum files "img:test.raw", "img2,raw" > and "img3.raw" invocation look like: >=20 > -drive file=3Dquorum:2/3:img\\:test.raw:img2,,raw:img3.raw > (note the double \\ and the double ,,) >=20 > Signed-off-by: Benoit Canet > + /* Get threshold */ > + errno =3D 0; > + s->threshold =3D strtoul(start, &a, 10); > + if (*a !=3D '/' || errno) { > + return -EINVAL; > + } > + a++; Hmm - you can fail to reject file=3Dquorum:/3:... (strtoul happily parses= a to 0 in that case, and is not required to set errno). But see below...= > + > + /* Get total */ > + errno =3D 0; > + s->total =3D strtoul(a, &b, 10); > + if (*b !=3D ':' || errno) { > + return -EINVAL; > + } > + b++; Again, you fail to reject file=3Dqourum: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; > + } =2E..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) !=3D s->total) { > + ret =3D -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. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2OILSQECHKEPIAVQMQQIC Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJQ+HdwAAoJEKeha0olJ0Nq7CoH/1YiebsW6WSMyxK55wy7q2OC 2ndrB6Uto3bBXDB0HqmqJHH2s/xRDy8uOjMFLi++8TE5OjJ5g4dJqCpIB9kL5U57 pxxalxWo7yU3ki3FyCeYl+VpAAXvisxr6aHmf9/3gvwmC6O7g/0EmDffWx55QfsS 1NbqnkN2W/Z6Yd5kzjAaLsYTdjr8rVaHSkKDqyGINB5ykNqJKQxo8i5aGnh2i1lI +BjiMAez4s3x0/E2vvoTp+HDJSDqk7U4n8Zww1LmMqpaIRBKu5EzdQ46G+FAIoS7 f6r+GLYYuqN6Sb4/Fh4jyRJj/stvQTOxOmlGqyXq2Qgixkk3ngvoFfrBCSSb+CU= =ZFl8 -----END PGP SIGNATURE----- ------enig2OILSQECHKEPIAVQMQQIC--