[ovs-dev] [PATCH] ovsdb: Fix memory leak in execute_update.

[William Tu]

Hi zhangtonghao,

Thanks for the patch!
I think it's safe to call free(ptr) when ptr == NULL. The free() will
be no operation.

Regards,
William

On Tue, Jul 26, 2016 at 9:18 PM, nickcooper-zhangtonghao
<nickcooper-zhangtonghao at opencloud.tech> wrote:
> Hi William,
> I reviewed your patch codes and found other bugs.
>
> If the ‘ovsdb_condition_from_json’ return the error, cnd->clauses will be
> set NULL, so ‘ovsdb_condition_destroy’ should check the 'cnd->clauses’ before
> free it.
>
> It is applied to ‘ovsdb_row_from_json’.
>
>
> Signed-off-by: nickcooper-zhangtonghao <nickcooper-zhangtonghao at opencloud.tech>
> ---
>  ovsdb/column.c      | 5 ++++-
>  ovsdb/condition.c   | 6 +++++-
>  ovsdb/replication.c | 3 +++
>  3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/ovsdb/column.c b/ovsdb/column.c
> index 8838df3..f01301f 100644
> --- a/ovsdb/column.c
> +++ b/ovsdb/column.c
> @@ -129,7 +129,10 @@ ovsdb_column_set_init(struct ovsdb_column_set *set)
>  void
>  ovsdb_column_set_destroy(struct ovsdb_column_set *set)
>  {
> -    free(set->columns);
> +    if (set->columns) {
> +        free(set->columns);
> +        set->columns = NULL;
> +    }
>  }
>
>  void
> diff --git a/ovsdb/condition.c b/ovsdb/condition.c
> index 6da3b08..f337a37 100644
> --- a/ovsdb/condition.c
> +++ b/ovsdb/condition.c
> @@ -485,7 +485,11 @@ ovsdb_condition_destroy(struct ovsdb_condition *cnd)
>      for (i = 0; i < cnd->n_clauses; i++) {
>          ovsdb_clause_free(&cnd->clauses[i]);
>      }
> -    free(cnd->clauses);
> +
> +    if (cnd->clauses) {
> +        free(cnd->clauses);
> +        cnd->clauses = NULL;
> +    }
>      cnd->n_clauses = 0;
>
>      ovsdb_condition_optimize_destroy(cnd);
> diff --git a/ovsdb/replication.c b/ovsdb/replication.c
> index 52b7085..bfd2ca1 100644
> --- a/ovsdb/replication.c
> +++ b/ovsdb/replication.c
> @@ -568,6 +568,8 @@ execute_delete(struct ovsdb_txn *txn, const char *uuid,
>      }
>
>      ovsdb_condition_destroy(&condition);
> +    json_destroy(CONST_CAST(struct json *, where));
> +
>      return error;
>  }
>
> @@ -625,6 +627,7 @@ execute_update(struct ovsdb_txn *txn, const char *uuid,
>      ovsdb_row_destroy(row);
>      ovsdb_column_set_destroy(&columns);
>      ovsdb_condition_destroy(&condition);
> +    json_destroy(CONST_CAST(struct json *, where));
>
>      return error;
>  }
> --
> 1.8.3.1
>
>
>> From: William Tu <u9012063 at gmail.com>
>> To: dev at openvswitch.org
>> Subject: [ovs-dev] [PATCH] ovsdb: Fix memory leak in execute_update.
>> Message-ID: <1469582910-64371-1-git-send-email-u9012063 at gmail.com>
>>
>> Valgrind testcase 1804 ovsdb-server.at:1023 insert rows, update rows by value
>> reports the following leak.
>>    json_from_string (json.c:1025)
>>    execute_update (replication.c:614), similarily at execute_delete()
>>    process_table_update (replication.c:502)
>>    process_notification.part.5 (replication.c:445)
>>    process_notification (replication.c:402)
>>    check_for_notifications (replication.c:418)
>>    replication_run (replication.c:110)
>>
>> Signed-off-by: William Tu <u9012063 at gmail.com>
>> ---
>> ovsdb/replication.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/ovsdb/replication.c b/ovsdb/replication.c
>> index af7ae5c..fe89d39 100644
>> --- a/ovsdb/replication.c
>> +++ b/ovsdb/replication.c
>> @@ -573,6 +573,8 @@ execute_delete(struct ovsdb_txn *txn, const char *uuid,
>>     }
>>
>>     ovsdb_condition_destroy(&condition);
>> +    json_destroy(CONST_CAST(struct json *, where));
>> +
>>     return error;
>> }
>>
>> @@ -630,6 +632,7 @@ execute_update(struct ovsdb_txn *txn, const char *uuid,
>>     ovsdb_row_destroy(row);
>>     ovsdb_column_set_destroy(&columns);
>>     ovsdb_condition_destroy(&condition);
>> +    json_destroy(CONST_CAST(struct json *, where));
>>
>>     return error;
>> }
>> --
>> 2.5.0
>