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

Ben Pfaff blp at ovn.org
Fri Jun 15 20:03:06 UTC 2018


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?

Thanks,

Ben.


More information about the dev mailing list