[ovs-dev] [PATCH ovn v2 2/2] lflow.c: Release ref_lflow_node as soon as it is not needed.

Han Zhou hzhou at ovn.org
Wed Sep 16 18:36:01 UTC 2020


On Wed, Sep 16, 2020 at 10:51 AM Dumitru Ceara <dceara at redhat.com> 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
>


More information about the dev mailing list