> On Dec 21, 2017, at 4:30 PM, Ben Pfaff <b...@ovn.org> 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 <jpet...@ovn.org> --Justin _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev