On Mon, Apr 13, 2015 at 05:56:15PM -0700, Joe Stringer wrote:
> Signed-off-by: Joe Stringer <joestrin...@nicira.com>

The code in scan_u128() looks wrong to me: I don't see anything that
makes the second call to ovs_scan(), to get the mask, skip past the
value, e.g. by passing s + n to the second ovs_scan() or by advancing s
with s += n.

I'm also not sure that omitting the check for an exact-length 32-hexit
ufid is a good idea.  If I use 31 hexits instead then I think the
semantics of ovs_scan() will yield 0-bits in the upper nibble of the .lo
member of ovs_u128, which doesn't seem to make sense.

(That tends to argue that U128_SCAN_FMT is a bad idea entirely.)

In the past we've tried to make sure that hexadecimal values are always
preceded by 0x, so that more systematic parsing of output might be
possible in the future.  Have you considered that for ufids?

I have another idea too: these issues for ovs_u128 are quite similar to
the issues for UUIDs, which are also exactly 128 bits long and already
have support for parsing, formatting, and so on, and furthermore have a
distinctive format that is easily identifiable.  Have you considered
using UUIDs as the representation for ufids?

Thanks,

Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to