On Mon, Sep 08, 2014 at 05:06:05PM -0700, Ben Pfaff wrote:
> On Fri, Sep 05, 2014 at 04:05:13PM -0700, Jarno Rajahalme wrote:
> > When a whole field of a key value is ignored, skip it when formatting
> > the key, and allow it to be left out when parsing the key from a
> > string. However, when the unmasked bits have non-zero values (as in
> > keys received from a datapath), or when the 'verbose' formatting is
> > requested those are still formatted, as it may help in debugging.
> >
> > Now the named key fields can also be given in arbitrary order.
> > Duplicate field values are not checked for, so the last one will
> > remain in effect.
> >
> > Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
>
> This makes the formatting and parsing code less disastery. Thank you.
...
> I only made it as far as the SCAN_PUT_ATTR macro. I'll continue the
> review from there tomorrow.
I noticed that a lot of the macros could better protect their
arguments, e.g. in the definition of SCAN_PUT_ATTR, parentheses around
ATTR here:
+ if (ATTR == OVS_KEY_ATTR_TUNNEL) { \
and around DATA here:
+ tun_key_to_attr(BUF, (const struct flow_tnl *)(void *)&DATA); \
+ } else { \
and ATTR and DATA (twice) here:
+ nl_msg_put_unspec(BUF, ATTR, &DATA, sizeof DATA); \
+ }
I am not sure whether this is the approach that I would have ended up
with, but whether yes or no, it is a huge improvement in
maintainability. Thank you.
Acked-by: Ben Pfaff <b...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
