[ovs-dev] [PATCH] ovsdb-idl: Remove unnecessary code in track clear.

Han Zhou zhouhan at gmail.com
Fri Jun 15 20:38:02 UTC 2018


On Fri, Jun 15, 2018 at 1:03 PM, Ben Pfaff <blp at ovn.org> wrote:
>
> On Wed, May 30, 2018 at 10:08:26AM -0700, Han Zhou wrote:
> > In ovsdb_idl_db_track_clear(), it needs to free the deleted row.
> > However, it unnecessary to call ovsdb_idl_row_clear_old(), because
> > this has been called in ovsdb_idl_row_destroy(). It is also confusing
> > because it is called only if:
> >     if (ovsdb_idl_row_is_orphan(row))
> > This is contradict with the check in ovsdb_idl_row_clear_old():
> >     if (!ovsdb_idl_row_is_orphan(row))
> >
> > Signed-off-by: Han Zhou <hzhou8 at ebay.com>
> > ---
> >  lib/ovsdb-idl.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> > index c6ff78e..958f924 100644
> > --- a/lib/ovsdb-idl.c
> > +++ b/lib/ovsdb-idl.c
> > @@ -1710,7 +1710,6 @@ ovsdb_idl_db_track_clear(struct ovsdb_idl_db *db)
> >                  ovs_list_remove(&row->track_node);
> >                  ovs_list_init(&row->track_node);
> >                  if (ovsdb_idl_row_is_orphan(row)) {
> > -                    ovsdb_idl_row_clear_old(row);
> >                      free(row);
> >                  }
> >              }
>
> Thanks for looking at this code.
>
> This code is definitely useless since, as you say,
> ovsdb_idl_row_clear_old() does nothing for orphan rows.
>
> I understand the tracking feature very poorly.  This change implies that
> a tracked row never has data, since it never gets freed here.  Or maybe
> it implies that any data in a tracked row is getting leaked.  Do you
> understand which or those is true?

Hi Ben,

Currently the tracked row doesn't maintain any data, so there is no leak.
Here is the later patch that tries to keep the data until track_clear:
https://patchwork.ozlabs.org/patch/924268/, but it is independent of this
change.

Thanks,
Han


More information about the dev mailing list