On Tue, Apr 22, 2014 at 06:27:18PM +0100, Zoltan Kiss wrote:
> This function returns true when 's' is negative or greater than UINT_MAX.
> Also,
> the representation of 'int' and 'unsigned int' is implementation dependent, so
> converting [INT_MAX..UINT_MAX] values with str_to_int is fragile.
> Instead, we should convert straight to 'long long' and do a boundary check
> before returning the converted value.
>
> Signed-off-by: Zoltan Kiss <zoltan.k...@citrix.com>
It's implementation dependent in theory but in practice OVS only runs
on two's complement systems. The intent here was to accept negative
or out of range values and adjust them to lie within the range as if
by truncation of the two's complement bitwise pattern. This commit
changes the semantics.
However, all of the existing users of str_to_uint() would be better
off with the semantics that you propose. Let's change it.
I think that we'd be better off with this in the .c file now. It was
only in the .h file because it was completely trivial.
I think that the other str_to_u*() functions should adopt the new
semantics too. Or we could delete them, since they have no users.
Here, I would remove the OVS_UNLIKELY, not because it is wrong but
because this is not on any fast path and such annotations make code
harder to read. I would also remove the inner ():
> + if (OVS_UNLIKELY(!ok || (ll < 0) || (ll > UINT_MAX))) {
> + *u = 0;
> + return false;
> + }
> + else {
> + *u = ll;
> + return true;
> + }
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
