On 02/14/2017 04:26 AM, Markus Armbruster wrote: > Change the qemu_strtosz() & friends to return -EINVAL when @endptr is > null and the conversion doesn't consume the string completely. > Matches how qemu_strtol() & friends work. > > Only test_qemu_strtosz_simple() passes a null @endptr. No functional > change there, because its conversion consumes the string. > > Simplify callers that use @endptr only to fail when it doesn't point > to '\0' to pass a null @endptr instead. > > Cc: Dr. David Alan Gilbert <dgilb...@redhat.com> > Cc: Eduardo Habkost <ehabk...@redhat.com> (maintainer:X86) > Cc: Kevin Wolf <kw...@redhat.com> (supporter:Block layer core) > Cc: Max Reitz <mre...@redhat.com> (supporter:Block layer core) > Cc: qemu-bl...@nongnu.org (open list:Block layer core) > Signed-off-by: Markus Armbruster <arm...@redhat.com> > --- > hmp.c | 6 ++---- > hw/misc/ivshmem.c | 6 ++---- > qapi/opts-visitor.c | 5 ++--- > qemu-img.c | 7 +------ > qemu-io-cmds.c | 7 +------ > target/i386/cpu.c | 5 ++--- > tests/test-cutils.c | 6 ++++++ > util/cutils.c | 14 +++++++++----- > 8 files changed, 25 insertions(+), 31 deletions(-)
Nice cleanup. Reviewed-by: Eric Blake <ebl...@redhat.com> > +++ b/qemu-img.c > @@ -370,14 +370,9 @@ static int add_old_style_options(const char *fmt, > QemuOpts *opts, > > static int64_t cvtnum(const char *s) > { > +++ b/qemu-io-cmds.c > @@ -137,14 +137,9 @@ static char **breakline(char *input, int *count) > > static int64_t cvtnum(const char *s) > { > - char *end; Why do we reimplement cvtnum() as copied static functions instead of exporting it? But that would be a separate cleanup (perhaps squashed into 20/24, where you use cvtnum in qemu-img). > @@ -217,7 +217,8 @@ static int64_t do_strtosz(const char *nptr, char **end, > errno = 0; > val = strtod(nptr, &endptr); > if (isnan(val) || endptr == nptr || errno != 0) { Hmm - we explicitly reject "NaN", but not "infinity". But when strtod() accepts infinity, ... > - goto fail; > + retval = -EINVAL; > + goto out; > } > fraction = modf(val, &integral); then modf() returns 0 with integral left at infinity... > if (fraction != 0) { > @@ -232,17 +233,20 @@ static int64_t do_strtosz(const char *nptr, char **end, > assert(mul >= 0); > } > if (mul == 1 && mul_required) { > - goto fail; > + retval = -EINVAL; > + goto out; > } > if ((val * mul >= INT64_MAX) || val < 0) { ...and the multiply exceeds INT64_MAX, so we still end up rejecting it (with ERANGE instead of EINVAL). Weird way but seems to work, and is pre-existing, so not this patch's problem. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature