Re: [Qemu-devel] [PATCH 00/16] introduce OptsVisitor, rebase -net/-netdev parsing

[Laszlo Ersek]

On 06/06/12 17:16, Michael Roth wrote:
> On Wed, Jun 06, 2012 at 04:10:44PM +0200, Paolo Bonzini wrote:

>> The uintXX visitors do not fail if you pass a negative value.  I'm fine
>> with including the patch with the small bug and fixing it as a
>> follow-up, there's plenty of time before 1.2.
> 
> How would we implement such a check?
> 
> In the case of uint64_t, the field we're visiting is passed in as a
> uint64_t*, so -1 is indistinguishable from the unsigned interpretation
> of the field, which is within the valid range. 
> 
> For uintXX_t where XX < 64, a negative value would exceed the UINTXX_MAX
> check, so those cases are already handled.
> 
> Or am I missing something?

I found three instances of the patch on the list:

  http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg00333.html
  http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg01292.html
  http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg04068.html

looking at the third one, all of

- visit_type_uint8()
- visit_type_uint16()
- visit_type_uint32()
- visit_type_uint64()

seem to define "value" as an int64_t. Thus when we fall back to
(*v->type_int)(), the comparison is still done against an int64_t. Since
"int" is equivalent to "int32_t" on the platforms I can think of, and
"int64_t" to "long long", the comparisons are evaluated as follows:

  value > UINT8_MAX
  value > UINT16_MAX

First the right hand sides are promoted to "int" (with unchanged value),
and then "int" is converted to "long long" (both signed, different
conversion rank).

  value > UINT32_MAX

The right hand side is directly converted to "long long" (signed vs.
unsigned, signed has greater rank and can represent all values of the
lower-rank unsigned type).

I propose

  value < 0 || value > UINT8_MAX
  value < 0 || value > UINT16_MAX
  value < 0 || value > UINT32_MAX
  value < 0

Laszlo