On 09/25/2015 06:39 AM, Andreas Färber wrote: > All integers would get parsed by strtoll(), not handling the case of > UINT64 properties with the most significient bit set. > > Implement a .type_uint64 visitor callback, reusing the existing > parse_str() code through a new argument, using strtoull(). > > As a bug fix, ignore warnings about preference of qemu_strto[u]ll(). > > Cc: qemu-sta...@nongnu.org > Signed-off-by: Andreas Färber <afaer...@suse.de> > --- > qapi/string-input-visitor.c | 57 > +++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 52 insertions(+), 5 deletions(-) >
> @@ -50,7 +50,11 @@ static void parse_str(StringInputVisitor *siv, Error
> **errp)
>
> do {
> errno = 0;
> - start = strtoll(str, &endptr, 0);
> + if (u64) {
> + start = strtoull(str, &endptr, 0);
accepts the range [-ULLONG_MAX, ULLONG_MAX] (with 2s complement
wraparound). Do you really want -1 being a synonym for ULLONG_MAX, or do
you want to explicitly reject leading '-' when parsing unsigned
(arguments can be made for both behaviors; in fact, libvirt has two
separate wrappers for parsing uint64_t depending on which behavior is
wanted)
> + } else {
> + start = strtoll(str, &endptr, 0);
accepts the range [LLONG_MIN, LLONG_MAX] (that is, roughly half the
range of the unsigned version)
> + }
> if (errno == 0 && endptr > str) {
> if (*endptr == '\0') {
> cur = g_malloc0(sizeof(*cur));
> @@ -60,7 +64,7 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> range_compare);
> cur = NULL;
> str = NULL;
> - } else if (*endptr == '-') {
> + } else if (*endptr == '-' && !u64) {
Why do you not want to handle ranges when using unsigned numbers?
>
> +static void parse_type_uint64(Visitor *v, uint64_t *obj, const char *name,
> + Error **errp)
> +{
> + StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> +
> + if (!siv->string) {
> + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> + "integer");
> + return;
> + }
...
That's a lot of copy-and-paste. Can't you make parse_type_int64() and
parse_type_uint64() both call into a single helper method, that contains
the guts of the existing parse_type_int64() and adds a single parameter
for the one place where the two functions differ on their call to
parse_str()?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
