Re: [Qemu-devel] [PATCH v2] virtio-rng: Add human-readable error message for negative max-bytes parameter

[Eric Blake]

On 07/21/2014 02:13 PM, John Snow wrote:

> I can certainly grep through the code to find out who is using unsigned
> properties. In the case of uint32, -1 I believe will already wrap around
> but then overflow (because we parse as uint64_t) and throw an error, so
> I don't expect we will see anyone using -1 to signify "MAX" for less
> than 64bit properties. In the case of uint64, it may be more difficult
> to see who, if anyone, is abusing such behavior.

Actually, you may find that behavior on uint32 is MORE likely to be
confused, rather than less.  The _reason_ libvirt started tightening up
is because we hit a case where we were parsing an unsigned 32-bit
integer, but had different behavior on 32-bit hosts than on 64-bit
hosts, and it all boiled down to type promotion rules (basically,
strtoul("-1") on 32-bit platforms wrapped around to a 32-bit value,
which was still in range for uint32, while strtoul("-1") on 64-bit
platforms wrapped around to a 64-bit value which then appeared different
when truncated to uint32).  At least strtoull("-1") behaves the same on
both 32-bit and 64-bit hosts.

> 
> However, from a quick look-see it looks like DEFINE_PROP_UINT64 is used
> in 26 places. The fourth argument is "default value" and you can see
> many authors using -1 here, so either these authors expect wraparound or
> are trying to set the default value to something invalid that they will
> try to catch later on somehow.
> 
> CC'ing Eric Blake again for input, since he went through a similar
> ordeal recently and might have some input.

Tightening semantics is always a pain - in libvirt, we had to audit all
callers and make a case-by-case judgment call on whether the tighter
semantics of rejecting negatives made sense.  We ended up with very few
callers that still allowed wraparound, but there's no magic fix short of
just auditing the callers.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature