| Message ID | 20170317183835.26684-2-joe@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Mar 17, 2017 at 11:38:35AM -0700, Joe Stringer wrote: > It's slightly more cognitive load to read an inverse condition and then > invert it again to understand the 'else' condition. > > Signed-off-by: Joe Stringer <joe@ovn.org> It's also somewhat odd to handle the error condition before the common case--I think that CodingStyle actually has something to say about this--so maybe we should instead flip 'limited' to become its inverse 'below_limit' (and then rename the parameter to below_limitp or below_limit_). But if you like this better it's not a big deal to me, so: Acked-by: Ben Pfaff <blp@ovn.org>
On 17 March 2017 at 16:06, Ben Pfaff <blp@ovn.org> wrote: > On Fri, Mar 17, 2017 at 11:38:35AM -0700, Joe Stringer wrote: >> It's slightly more cognitive load to read an inverse condition and then >> invert it again to understand the 'else' condition. >> >> Signed-off-by: Joe Stringer <joe@ovn.org> > > It's also somewhat odd to handle the error condition before the common > case--I think that CodingStyle actually has something to say about > this--so maybe we should instead flip 'limited' to become its inverse > 'below_limit' (and then rename the parameter to below_limitp or > below_limit_). > > But if you like this better it's not a big deal to me, so: > Acked-by: Ben Pfaff <blp@ovn.org> Thanks, I took your suggestion and applied to master.
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index c6d83d4e6b29..e196597f4013 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -5118,15 +5118,15 @@ ofproto_flow_mod_learn(struct ofproto_flow_mod *ofm, bool keep_ref, rule_criteria_destroy(&criteria); } - if (!limited) { + if (limited) { + ofproto_flow_mod_uninit(ofm); + } else { ofm->version = rule->ofproto->tables_version + 1; error = ofproto_flow_mod_learn_start(ofm); if (!error) { ofproto_flow_mod_learn_finish(ofm, NULL); } - } else { - ofproto_flow_mod_uninit(ofm); } ovs_mutex_unlock(&ofproto_mutex);
It's slightly more cognitive load to read an inverse condition and then invert it again to understand the 'else' condition. Signed-off-by: Joe Stringer <joe@ovn.org> --- ofproto/ofproto.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)