Skip to content
  • Eric Blake's avatar
    896fbd90
    cutils: Set value in all qemu_strtosz* error paths · 896fbd90
    Eric Blake authored
    
    
    Making callers determine whether or not *value was populated on error
    is not nice for usability.  Pre-patch, we have unit tests that check
    that *result is left unchanged on most EINVAL errors and set to 0 on
    many ERANGE errors.  This is subtly different from libc strtoumax()
    behavior which returns UINT64_MAX on ERANGE errors, as well as
    different from our parse_uint() which slams to 0 on EINVAL on the
    grounds that we want our functions to be harder to mis-use than
    strtoumax().
    
    Let's audit callers:
    
    - hw/core/numa.c:parse_numa() fixed in the previous patch to check for
      errors
    
    - migration/migration-hmp-cmds.c:hmp_migrate_set_parameter(),
      monitor/hmp.c:monitor_parse_arguments(),
      qapi/opts-visitor.c:opts_type_size(),
      qapi/qobject-input-visitor.c:qobject_input_type_size_keyval(),
      qemu-img.c:cvtnum_full(), qemu-io-cmds.c:cvtnum(),
      target/i386/cpu.c:x86_cpu_parse_featurestr(), and
      util/qemu-option.c:parse_option_size() appear to reject all failures
      (although some with distinct messages for ERANGE as opposed to
      EINVAL), so it doesn't matter what is in the value parameter on
      error.
    
    - All remaining callers are in the testsuite, where we can tweak our
      expectations to match our new desired behavior.
    
    Advancing to the end of the string parsed on overflow (ERANGE), while
    still returning 0, makes sense (UINT64_MAX as a size is unlikely to be
    useful); likewise, our size parsing code is complex enough that it's
    easier to always return 0 when endptr is NULL but trailing garbage was
    found, rather than trying to return the value of the prefix actually
    parsed (no current caller cared about the value of the prefix).
    
    Signed-off-by: default avatarEric Blake <eblake@redhat.com>
    Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
    Message-Id: <20230522190441.64278-16-eblake@redhat.com>
    896fbd90
    cutils: Set value in all qemu_strtosz* error paths
    Eric Blake authored
    
    
    Making callers determine whether or not *value was populated on error
    is not nice for usability.  Pre-patch, we have unit tests that check
    that *result is left unchanged on most EINVAL errors and set to 0 on
    many ERANGE errors.  This is subtly different from libc strtoumax()
    behavior which returns UINT64_MAX on ERANGE errors, as well as
    different from our parse_uint() which slams to 0 on EINVAL on the
    grounds that we want our functions to be harder to mis-use than
    strtoumax().
    
    Let's audit callers:
    
    - hw/core/numa.c:parse_numa() fixed in the previous patch to check for
      errors
    
    - migration/migration-hmp-cmds.c:hmp_migrate_set_parameter(),
      monitor/hmp.c:monitor_parse_arguments(),
      qapi/opts-visitor.c:opts_type_size(),
      qapi/qobject-input-visitor.c:qobject_input_type_size_keyval(),
      qemu-img.c:cvtnum_full(), qemu-io-cmds.c:cvtnum(),
      target/i386/cpu.c:x86_cpu_parse_featurestr(), and
      util/qemu-option.c:parse_option_size() appear to reject all failures
      (although some with distinct messages for ERANGE as opposed to
      EINVAL), so it doesn't matter what is in the value parameter on
      error.
    
    - All remaining callers are in the testsuite, where we can tweak our
      expectations to match our new desired behavior.
    
    Advancing to the end of the string parsed on overflow (ERANGE), while
    still returning 0, makes sense (UINT64_MAX as a size is unlikely to be
    useful); likewise, our size parsing code is complex enough that it's
    easier to always return 0 when endptr is NULL but trailing garbage was
    found, rather than trying to return the value of the prefix actually
    parsed (no current caller cared about the value of the prefix).
    
    Signed-off-by: default avatarEric Blake <eblake@redhat.com>
    Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
    Message-Id: <20230522190441.64278-16-eblake@redhat.com>
Loading