On Mon, May 04, 2015 at 06:09:12PM -0700, Justin Pettit wrote:
>
> > On May 1, 2015, at 4:17 PM, Ben Pfaff <b...@nicira.com> wrote:
> >
> >
> > +/* Adds a flow with the specified 'match' and 'actions' to the OpenFlow
> > table
> > + * numbered 'table_id' with the given 'priority'.
> > + *
> > + * This just assembles the desired flow table in memory. Nothing is
> > actually
> > + * sent to the switch until a later call to ofctrl_run(). */
> > +void
> > +ofctrl_add_flow(uint8_t table_id, uint16_t priority,
> > + const struct match *match, const struct ofpbuf *ofpacts)
>
> The function and comment refer to the supplied actions differently
> ("ofpacts" vs "actions"). I think "actions" may be a better name,
> since it's going into an array of type "ofpact", which is a little
> confusing.
Do you mean it's confusing because it's going into an array named
'ofpacts', because then a single function refers to two entities named
ofpacts? I don't see why putting data of a given type into an array
named for that type is confusing; I usually think of it as helpful.
Anyway, I renamed the parameter to 'actions'.
> Do you think it's worth saying that ownership of the arguments is
> maintained by the caller?
I usually think of that as implied, for 'const' arguments.
I added a sentence to say it.
> Acked-by: Justin Pettit <jpet...@nicira.com>
Thank you for the review!
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
