Re: [PULL 09/21] cutils: Adjust signature of parse_uint[_full]

[Markus Armbruster]

Sorry for being late to the party...

Eric Blake <eblake@redhat.com> writes:

> It's already confusing that we have two very similar functions for
> wrapping the parse of a 64-bit unsigned value, differing mainly on
> whether they permit leading '-'.  Adjust the signature of parse_uint()
> and parse_uint_full() to be like all of qemu_strto*(): put the result
> parameter last, use the same types (uint64_t and unsigned long long
> have the same width, but are not always the same type), and mark
> endptr const (this latter change only affects the rare caller of
> parse_uint).  Adjust all callers in the tree.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
> Message-Id: <20230522190441.64278-8-eblake@redhat.com>

[...]

> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 587f31baf6b..8812d23677a 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -454,8 +454,8 @@ opts_type_uint64(Visitor *v, const char *name, uint64_t 
> *obj, Error **errp)
>      OptsVisitor *ov = to_ov(v);
>      const QemuOpt *opt;
>      const char *str;
> -    unsigned long long val;
> -    char *endptr;
> +    uint64_t val;

val changes from unsigned long long, which is at least 64 bits, to
uint64_t, which is exactly 64 bits.

> +    const char *endptr;
>
>      if (ov->list_mode == LM_UNSIGNED_INTERVAL) {
>          *obj = ov->range_next.u;
> @@ -471,17 +471,17 @@ opts_type_uint64(Visitor *v, const char *name, uint64_t 
> *obj, Error **errp)
>      /* we've gotten past lookup_scalar() */
>      assert(ov->list_mode == LM_NONE || ov->list_mode == LM_IN_PROGRESS);
>
> -    if (parse_uint(str, &val, &endptr, 0) == 0 && val <= UINT64_MAX) {
> +    if (parse_uint(str, &endptr, 0, &val) == 0 && val <= UINT64_MAX) {

val <= UINT64_MAX is now useless, isn't it?

>          if (*endptr == '\0') {
>              *obj = val;
>              processed(ov, name);
>              return true;
>          }
>          if (*endptr == '-' && ov->list_mode == LM_IN_PROGRESS) {
> -            unsigned long long val2;
> +            uint64_t val2;

val2 changes from unsigned long long, which is at least 64 bits, to
uint64_t, which is exactly 64 bits.

>              str = endptr + 1;
> -            if (parse_uint_full(str, &val2, 0) == 0 &&
> +            if (parse_uint_full(str, 0, &val2) == 0 &&
>                  val2 <= UINT64_MAX && val <= val2 &&

val2 <= UINT64_MAX is now useless, isn't it?

>                  val2 - val < OPTS_VISITOR_RANGE_MAX) {
>                  ov->range_next.u = val;

[...]