On Sat, Jan 07, 2012 at 11:11:56AM -0800, Ethan Jackson wrote:
> This will ease the implementation of future patches.
>
> Signed-off-by: Ethan Jackson <et...@nicira.com>
Looks good. I think that this will become more useful over time.
In ofp_print_packet_in(), the use of ~ (twice) below makes me nervous.
Testing the result of ~ for a nonzero value is risky, because C's
promotion rules mean that it will always be true due if the expression's
type is narrower than "int". That should not ever be the case here, but
I think I still would prefer explicit comparisons against
e.g. htonll(UINT64_MAX), because it requires less thinking to see that
it is correct:
> + if (pin.fmd.tun_id_mask) {
> + ds_put_format(string, " tun_id=0x%"PRIx64, ntohll(pin.fmd.tun_id));
> + if (~pin.fmd.tun_id_mask) {
> + ds_put_format(string, "/0x%"PRIx64, ntohll(pin.fmd.tun_id_mask));
> + }
> + }
> +
> + for (i = 0; i < FLOW_N_REGS; i++) {
> + if (pin.fmd.reg_masks[i]) {
> + ds_put_format(string, " reg%d=0x%"PRIx32, i, pin.fmd.regs[i]);
> + if (~pin.fmd.reg_masks[i]) {
> + ds_put_format(string, "/0x%"PRIx32, pin.fmd.reg_masks[i]);
> + }
> + }
> + }
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
