On Wed, Sep 16, 2020 at 10:51 AM Dumitru Ceara <[email protected]> wrote: > > On 9/16/20 2:18 PM, Dumitru Ceara wrote: > > [...] > > > > >> + hmap_remove(&lfrr->ref_lflow_table, &lrln->rlfn->node); > >> + ref_lflow_node_destroy(lrln->rlfn); > > > > As mentioned on v1 (at least for me) it's really hard to keep track of > > what's going on due to the variable/field names, e.g., "lrln->rlfn". I'd > > really like it if we had somewhat more explicit names. > > > > This can be done as a follow up patch. I guess the goal is to fix the > crash as soon as possible. Refactoring should come afterwards. > Agree. For the names, what do you suggest? Here rlfn means ref_lflow_node, lrln means lflow_ref_list_node. It is common to use short abbreviations for variables with local scope, but maybe the rules are not clear enough. Maybe lrln can be lfrln? Or maybe the name ref_lflow_node itself is confusing? 'ref' represents a resource which is identified by ref_type + ref_name. Let me know your suggestion.
For the comment, I hope my explains in 1/2 helped understanding it. The reason why the check was needed is basically what the code is telling: when the lflow_uuids is empty (no lflows referencing this resource). > Thanks, > Dumitru > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
