On 3 April 2017 at 13:30, Andy Zhou <az...@ovn.org> wrote: > When adding a new field in the 'struct dpif_backer_support', the > corresponding appctl show command should be updated to display > the new field. Currently, there is nothing to remind the developer > that to update the show command. This can lead to code maintenance > issues. > > Switch to use macros to define those fields. This makes the show > command update automatic. > > Signed-off-by: Andy Zhou <az...@ovn.org> > > ---
Looks like there's a few typos: 's/FILED/FIELD/g' A couple more comments below, but otherwise LGTM. Thanks! Acked-by: Joe Stringer <j...@ovn.org> > v1->v2: > Add more comments. Fix typos > --- > lib/odp-util.h | 52 ++++++++++++++++++++++++++------------------ > ofproto/ofproto-dpif.c | 28 ++++++++++-------------- > ofproto/ofproto-dpif.h | 59 > ++++++++++++++++++++++++++++++-------------------- > 3 files changed, 78 insertions(+), 61 deletions(-) > > diff --git a/lib/odp-util.h b/lib/odp-util.h > index 50fa1d133e1f..d9668a75e811 100644 > --- a/lib/odp-util.h > +++ b/lib/odp-util.h > @@ -167,30 +167,40 @@ int odp_flow_from_string(const char *s, > const struct simap *port_names, > struct ofpbuf *, struct ofpbuf *); > > +/* ODP_SUPPORT_FIELD(TYPE, FIELD_NAME, FILED_DESCRIPTION) > + * > + * Each 'ODP_SUPPORT_FIELD' defines a member in 'struct odp_support', > + * and represents support for related OVS_KEY_ATTR_* fields. > + * They are defined as macros so that 'dpif_show_support()' is less > + * likely to go out of sync when new fields are added. */ This is a bit simpler language, similarly for the dpif_support below. "They are defined as macros to keep 'dpif_show_support()' in sync as new fields are added." <snip> > +/* DPIF_SUPPORT_FILED(TYPE, FIELD_NAME, FIELD_DESCRIPTION) > + * > + * Each 'DPIF_SUPPORT_FILED' defines a member in 'struct dpif_backer_support' > + * and represents support for a datapath action. > + * They are defined as macros so that 'dpif_show_support()' is less > + * likely to go out of sync when new fields are added. */ > +#define DPIF_SUPPORT_FIELDS \ If DPIF_SUPPORT_FIELDS is about actions, perhaps a name like DPIF_SUPPORT_ACTION() is a better name? _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
