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 <[email protected]> > --- > 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? Thanks, Ben. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
