On Thu, Dec 21, 2017 at 04:54:28PM -0800, Justin Pettit wrote:
> 
> 
> > On Dec 21, 2017, at 4:30 PM, Ben Pfaff <[email protected]> wrote:
> > 
> > What do you think of this version?
> 
> Yes, it's much clearer to me.  Thanks!
> 
> > @@ -1458,10 +1466,12 @@ ovsdb_jsonrpc_monitor_cond_change(struct 
> > ovsdb_jsonrpc_session *s,
> >     }
> > 
> >     /* Change monitor id */
> > -    hmap_remove(&s->monitors, &m->node);
> > -    json_destroy(m->monitor_id);
> > -    m->monitor_id = json_clone(params->u.array.elems[1]);
> > -    hmap_insert(&s->monitors, &m->node, json_hash(m->monitor_id, 0));
> > +    if (changing_id) {
> > +        hmap_remove(&s->monitors, &m->node);
> > +        json_destroy(m->monitor_id);
> > +        m->monitor_id = json_clone(new_monitor_id);
> > +        hmap_insert(&s->monitors, &m->node, json_hash(m->monitor_id, 0));
> > +    }
> 
> It doesn't matter, but you can probably drop the comment, since the "if" 
> statement is pretty clear.
> 
> Acked-by: Justin Pettit <[email protected]>

Thanks, I removed the comment and applied this to master.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to