From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54108) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adg20-0005Hv-IL for qemu-devel@nongnu.org; Wed, 09 Mar 2016 10:28:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adg1z-0001u0-PR for qemu-devel@nongnu.org; Wed, 09 Mar 2016 10:28:12 -0500 References: <1455615450-15138-1-git-send-email-xiecl.fnst@cn.fujitsu.com> <1455615450-15138-3-git-send-email-xiecl.fnst@cn.fujitsu.com> <56DB21B7.7050104@redhat.com> <56DDA5FF.2050900@redhat.com> <56DDA630.4000409@redhat.com> <56DE3FAE.6000305@cn.fujitsu.com> From: Max Reitz Message-ID: <56E040FF.5010606@redhat.com> Date: Wed, 9 Mar 2016 16:27:59 +0100 MIME-Version: 1.0 In-Reply-To: <56DE3FAE.6000305@cn.fujitsu.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="SUi7FQGoTsHRn1hElCI6crrEsQpMjOCWv" Subject: Re: [Qemu-devel] [PATCH v10 2/3] quorum: implement bdrv_add_child() and bdrv_del_child() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Changlong Xie , Eric Blake , qemu devel , Alberto Garcia , Kevin Wolf , Stefan Hajnoczi Cc: qemu block , Jiang Yunhong , Dong Eddie , Markus Armbruster , "Dr. David Alan Gilbert" , Gonglei , zhanghailiang This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --SUi7FQGoTsHRn1hElCI6crrEsQpMjOCWv Content-Type: multipart/mixed; boundary="ap4WaKvISN0fUC9lbxWUCuBDQION2BNin" From: Max Reitz To: Changlong Xie , Eric Blake , qemu devel , Alberto Garcia , Kevin Wolf , Stefan Hajnoczi Cc: Markus Armbruster , "Dr. David Alan Gilbert" , Dong Eddie , Jiang Yunhong , Wen Congyang , qemu block , zhanghailiang , Gonglei Message-ID: <56E040FF.5010606@redhat.com> Subject: Re: [PATCH v10 2/3] quorum: implement bdrv_add_child() and bdrv_del_child() References: <1455615450-15138-1-git-send-email-xiecl.fnst@cn.fujitsu.com> <1455615450-15138-3-git-send-email-xiecl.fnst@cn.fujitsu.com> <56DB21B7.7050104@redhat.com> <56DDA5FF.2050900@redhat.com> <56DDA630.4000409@redhat.com> <56DE3FAE.6000305@cn.fujitsu.com> In-Reply-To: <56DE3FAE.6000305@cn.fujitsu.com> --ap4WaKvISN0fUC9lbxWUCuBDQION2BNin Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 08.03.2016 03:57, Changlong Xie wrote: > On 03/08/2016 12:02 AM, Max Reitz wrote: >> On 07.03.2016 17:02, Eric Blake wrote: >>> On 03/05/2016 11:13 AM, Max Reitz wrote: >>> >>>>> + index =3D atoi(child->name + 9); >>>> >>>> Optional: Assert absence of an error: >>>> >>> >>> Indeed, atoi() is worthless, because it cannot do error detection. >>> >>>> unsigned long index; >>>> char *endptr; >>>> >>>> index =3D strtoul(child->name + 9, &endptr, 10); >>>> assert(index >=3D 0 && !*endptr); >>> >>> Still incorrect; you aren't handling errno properly for detecting all= >>> errors. Even better is to use qemu_strtoul(), which already handles >>> proper error detection. >> >> Yeah, I keep forgetting that it returns ULONG_MAX on range error... >=20 > Yes, we should limit the range to INT_MAX. How do you like the followin= g > codes, i just steal it from xen_host_pci_get_value(). >=20 > int rc; > const char *endptr; > unsigned long value; >=20 > assert(!strncmp(child->name, "children.", 9)); > rc =3D qemu_strtoul(child->name + 9, &endptr, 10, &value); Passing NULL instead of &endptr will make qemu_strtoul() check that the string passed to it (child->name + 9) only consists of a number; which should be true here, so you can do that (pass NULL instead of &endptr). > if (!rc) { > assert(value <=3D INT_MAX); > index =3D value; > } else { > error_setg_errno(errp, -rc, "Failed to parse value '%s'", > child->name + 9); > return; > } You could simplify this as assert(!rc && value <=3D INT_MAX); index =3D value; (It should be impossible for qemu_strtoul() to return an error here, so an assert() is just as fine as a normal error.) And you could get rid of the index =3D value assignment by making index a= n unsigned long and replacing all instances of "value" by "index". Max >=20 > Thanks > -Xie >=20 >> >> Max >> >=20 >=20 --ap4WaKvISN0fUC9lbxWUCuBDQION2BNin-- --SUi7FQGoTsHRn1hElCI6crrEsQpMjOCWv Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJW4ED/AAoJEDuxQgLoOKytsd0H/2c58pmdPyUYNrEFB2+tDa6F JODK93QAg6UfXntNRpNindPt/zCmL7kSvZLwCr7WnWfsOKUUsPjhnplE5FyOGfi3 HmLOYgs0/LHK+eHHruXojHnLuqrH8UDlVlys/FrMsnEXnxMwxHdBrpKp4xu38wmy cDusN/eL2y8nC/DuMAsvcM/dCz3BR+Rk4gPccrbhwT1KurkEauUXtWomr50GTB4T Qqod5MEYbqYWJ3yu0NHNQjgk+R2YwAmfXHnbPl0iDaihJwaThoHEhA6a28l8cV4E 9cHzNdAyVlxkZkDggNqTuGcVCJx6Ax5xVb2DRRqsvz8Po55mgsT3MMncKniYesk= =HWpU -----END PGP SIGNATURE----- --SUi7FQGoTsHRn1hElCI6crrEsQpMjOCWv--