On Fri, Jun 06, 2014 at 11:11:57AM +0100, Thomas Graf wrote: > On 06/05/14 at 10:02pm, Ben Pfaff wrote: > > +/* By default, criteria initialized by rule_criteria_init() will match > > flows > > + * that are read-only, on the assumption that the collected flows won't be > > + * modified. This function to match only flows that are be modifiable. > ^^^^^^^^^^^^^^^^ > > > + * > > + * Specify 'override_readonly' as false in ordinary circumstances, true if > > the > > + * caller has special privileges that allow it to modify even "read-only" > > + * flows. */ > > +static void > > +rule_criteria_require_rw(struct rule_criteria *criteria, > > + bool override_readonly) > > +{ > > + criteria->include_readonly = override_readonly; > > I think this needs to be inverted as you pass True if only want > RW flows.
I think that it is correct, but it is confusing, and I could use advice on how to make it clear. There are three cases: * Flows will not be modified, so read-only flows should be included. rule_criteria_require_rw() isn't called at all in that case, and thus criteria->include_readonly is true. * Flows will be modified, so read-only flows should not be included. rule_criteria_require_rw(false) sets criteria->include_readonly to false. * Flows will be modified, but the caller has special privileges, so read-only flows should be included anyway. rule_criteria_require_rw(true) sets criteria->include_readonly to true. Can you (or anyone) suggest better naming or comment wording to explain this clearly? Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev