Good, it is unnecessary to check again.
> On Jul 28, 2016, at 2:13 AM, William Tu <u9012...@gmail.com> wrote:
>
> 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-zhangtong...@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-zhangtong...@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 <u9012...@gmail.com>
>>> To: dev@openvswitch.org
>>> Subject: [ovs-dev] [PATCH] ovsdb: Fix memory leak in execute_update.
>>> Message-ID: <1469582910-64371-1-git-send-email-u9012...@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 <u9012...@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
>>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
