On 07/21/2014 03:15 PM, Markus Armbruster wrote:
John Snow <js...@redhat.com> writes:
On 07/21/2014 01:33 PM, Markus Armbruster wrote:
John Snow <js...@redhat.com> writes:
On 07/21/2014 03:48 AM, Markus Armbruster wrote:
It certainly could check whether the value fits into uint64_t.
A quick peek at how string-input-visitor.c uses strtoll() makes me
cringe.
[...]
What I meant by that was to say that by the time a value was returned
to visit_type_uint64, the value has already been possibly converted
implicitly from a negative value, and we can't tell at this level if
that happened without re-inspecting the string we were passed. At that
point, why not just fix the string parsing mechanics one more layer
down in parse_type_int() -- or by creating another routine primitive;
i.e parse_type_uint.
As Eric Blake noted elsewhere in the thread, it would be nice to have
the ability to have three behaviors at the lowest level -- signed,
unsigned with wraparound, and unsigned strict. The biggest question in
my mind is how to add the property flag to allow authors to opt-in to
the unsigned with wraparound option, where the unsigned strict option
makes the most sense to me as a default.
Do we have a use case for silently mapping negative numbers to positive
ones?
Via Eric Blake, for cases where "-1" is a convenient shorthand for
"MAX" in lieu of writing out gibberish values like 4 billion or 18
quintillion. I don't know if anyone actually relies on this behavior,
but I don't know that they're not. I can easily imagine something like
--max-log-messages=-1, for instance.
That's a pretty horrid way to let people say "maximum, please".
But I accept a backward compatibility argument.
Semantically and from an end-user usability standpoint, I certainly
don't disagree. In almost all cases, --no-limit or --use-maximum or
similar explicit commands are more meaningful, but knowing whether or
not we need to support parsing negative integers for unsigned properties
will come later. Perhaps in our case we will be able to avoid supporting
such a case and force people to use semantically meaningful properties.
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.
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.
