From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58300) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TwGPP-0008O6-CD for qemu-devel@nongnu.org; Fri, 18 Jan 2013 13:11:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TwGPN-0006xw-TI for qemu-devel@nongnu.org; Fri, 18 Jan 2013 13:11:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39703) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TwGPN-0006xi-L7 for qemu-devel@nongnu.org; Fri, 18 Jan 2013 13:11:17 -0500 Message-ID: <50F9903F.8040609@redhat.com> Date: Fri, 18 Jan 2013 11:11:11 -0700 From: Eric Blake MIME-Version: 1.0 References: <1358531842-16752-1-git-send-email-ehabkost@redhat.com> In-Reply-To: <1358531842-16752-1-git-send-email-ehabkost@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2RVVLSJIQFMLQQUGFXXVQ" Subject: Re: [Qemu-devel] [PATCH 1/8 v5] cutils: unsigned int parsing functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Laszlo Ersek , Chegu Vinod , qemu-devel@nongnu.org, Anthony Liguori , Markus Armbruster This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2RVVLSJIQFMLQQUGFXXVQ Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 01/18/2013 10:57 AM, Eduardo Habkost wrote: > There are lots of duplicate parsing code using strto*() in QEMU, and > most of that code is broken in one way or another. Even the visitors > code have duplicate integer parsing code[1]. This introduces functions > to help parsing unsigned int values: parse_uint() and parse_uint_full()= =2E >=20 > Parsing functions for signed ints and floats will be submitted later. >=20 > parse_uint_full() has all the checks made by opts_type_uint64() at > opts-visitor.c: >=20 > - Check for NULL (returns -EINVAL) > - Check for negative numbers (returns -EINVAL) > - Check for empty string (returns -EINVAL) > - Check for overflow or other errno values set by strtoll() (returns > -errno) > - Check for end of string (reject invalid characters after number) > (returns -EINVAL) >=20 > parse_uint() does everything above except checking for the end of the > string, so callers can continue parsing the remainder of string after > the number. >=20 > Unit tests included. >=20 > [1] string-input-visitor.c:parse_int() could use the same parsing code > used by opts-visitor.c:opts_type_int(), instead of duplicating that= > logic. >=20 > Signed-off-by: Eduardo Habkost > --- > + * > + * If @s is null, or @base is invalid, or @s doesn't start with an= > + * integer in the syntax above, set *@value to 0, *@endptr to @s, = and > + * return -EINVAL. > + * > + * Set @endptr to point right beyond the parsed integer. > + * > + * If the integer overflows unsigned long long, set *@value to > + * ULLONG_MAX, and return -ERANGE. Is it worth explicitly mentioning that *@endptr is set past the last digit parsed in the -ERANGE case? It's implied that it was set beyond the parsed integer, but did you stop parsing the moment you detected overflow (and thus *endptr might still be pointing to a digit), or is it set beyond all possible digits to the first non-digit? > +/** > + * parse_uint_full: > + * > + * @s: String to parse > + * @value: Destination for parsed integer value > + * @base: integer base, between 2 and 36 inclusive, or 0 > + * > + * Parse unsigned integer from entire string > * > * Have the same behavior of parse_uint(), but with an additional = check > - * for additional data after the parsed number (in that case, the = function > - * will return -EINVAL). > + * for additional data after the parsed number. If extra character= s are present > + * after the parsed number, the function will return -EINVAL, and = the caller > + * should not rely on the value set on *@value. This says *value is unreliable; > */ > int parse_uint_full(const char *s, unsigned long long *value, int = base) > { > @@ -345,6 +360,7 @@ > return r; > } > if (*endp) { > + *value =3D 0; > return -EINVAL; while this says it is explicitly 0. Is this an intentional mismatch, especially given that parse_uint explicitly documents that *value is always set to a reliable value even on -EINVAL? > + /* make sure we reject negative numbers: */ > + sp =3D s; > + while (isspace((unsigned char)*sp)) { > + ++sp; > + } > + if (*sp =3D=3D '-') { > + r =3D -EINVAL; > + goto out; > + } > + > + errno =3D 0; > + val =3D strtoull(s, &endp, base); Is it worth a micro-optimization of calling strtoull(sp,...) instead os strtoull(s,...), to avoid reparsing all the space that we just skipped? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2RVVLSJIQFMLQQUGFXXVQ 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+ZA/AAoJEKeha0olJ0NqSi8IAIfTRgOl4SRRSJuqVamdulIN fKeOV1RShsLIUbQ8szedTpYW3rnQoCnpa50GQ2yvDRA4Zy/9GoguExZuJMl2hH0X XNIkedzV5BljYh8kG7L1wOo623orRKxHUakgJu/N6ucQZ8o6M7h92/mVmadmf/WI 34+Bq7MoPtSxyAA9HZkq4+3rhaICp3Dm5NnE2lmhymar6CSjZ+SzVt55wELS0FpT 7J/D51xKrc9BUYIZJUMo+AA+ML9Ys1SpaHYDkdSwqyYdVmjxuxISGf+TByWOfoCw a1ZcwJUTu5GD7vUw3pQV/wOmFk75e3NZ+x2u5TRljm/k2EDaa6OPOcuwDBqw/bY= =vs0w -----END PGP SIGNATURE----- ------enig2RVVLSJIQFMLQQUGFXXVQ--