Re: [Qemu-devel] [PATCH qom-next] qapi: exclude negative values in uint*_t Visitor interfaces

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On Wed, Jun 06, 2012 at 06:00:27PM +0200, Laszlo Ersek wrote:
> 
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> (To be applied on top of 0f2de4a8.)
> 
>  qapi/qapi-visit-core.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 9a29674..81f697f 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -106,7 +106,7 @@ void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
>          } else {
>              value = *obj;
>              v->type_int(v, &value, name, errp);
> -            if (value > UINT8_MAX) {
> +            if (value < 0 || value > UINT8_MAX) {
>                  error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
>                            "uint8_t");
>                  return;
> @@ -125,7 +125,7 @@ void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp
>          } else {
>              value = *obj;
>              v->type_int(v, &value, name, errp);
> -            if (value > UINT16_MAX) {
> +            if (value < 0 || value > UINT16_MAX) {
>                  error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
>                            "uint16_t");
>                  return;
> @@ -144,7 +144,7 @@ void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp
>          } else {
>              value = *obj;
>              v->type_int(v, &value, name, errp);
> -            if (value > UINT32_MAX) {
> +            if (value < 0 || value > UINT32_MAX) {
>                  error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
>                            "uint32_t");
>                  return;
> @@ -163,6 +163,11 @@ void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp
>          } else {
>              value = *obj;
>              v->type_int(v, &value, name, errp);
> +            if (value < 0) {
> +                error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
> +                          "uint64_t");
> +                return;
> +            }

Sorry, responded a bit late in the last thread. But 2^64-1 is a valid
value for visit_type_uint64(), yet due to being stored to a int64_t,
it will fail the < 0 check, which introduces a regression for an
acceptable use-case.

The other checks make sense, however, since the max values do not
exceed the max signed range of the intermediate int64_t we store to, so
they're not ambiguous.

>              *obj = value;
>          }
>      }
> -- 
> 1.7.1
> 
>