[ovs-dev] [PATCH 05/10] trigger: Free leaked ovsdb_schema

Yifeng Sun pkusunyifeng at gmail.com
Fri Sep 13 16:45:46 UTC 2019


Thanks Aginwala,

Could you please double check if 'json_destroy(result)' is necessary here?
If result != NULL, then it is passed in trigger_success(), which puts
result in 't->reply',
later, jsonrpc_msg_destroy() will free it.

Thanks,
Yifeng

On Thu, Sep 12, 2019 at 2:00 PM aginwala <aginwala at asu.edu> wrote:
>
> One minor suggestion here:
> Can you also handle freeing result:
> diff --git a/ovsdb/trigger.c b/ovsdb/trigger.c
> index 6f4ed96b0..0158957d6 100644
> --- a/ovsdb/trigger.c
> +++ b/ovsdb/trigger.c
> @@ -214,6 +214,7 @@ ovsdb_trigger_try(struct ovsdb_trigger *t, long long int now)
>                      /* Unsatisfied "wait" condition.  Take no action now, retry
>                       * later. */
>                  }
> +                json_destroy(result);
>                  return false;
>              }
>
> Else I can handle that in separate patch. Else, acked by for the series.
> Acked-by: Aliasgar Ginwala <aginwala at ebay.com>
>
> On Wed, Sep 11, 2019 at 2:19 PM Yifeng Sun <pkusunyifeng at gmail.com> wrote:
>>
>> Valgrind reported:
>>
>> 1925: schema conversion online - standalone
>>
>> ==10884== 689 (56 direct, 633 indirect) bytes in 1 blocks are definitely lost in loss record 384 of 420
>> ==10884==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==10884==    by 0x44A592: xcalloc (util.c:121)
>> ==10884==    by 0x40E2EC: ovsdb_schema_create (ovsdb.c:41)
>> ==10884==    by 0x40E688: ovsdb_schema_from_json (ovsdb.c:217)
>> ==10884==    by 0x416C6F: ovsdb_trigger_try (trigger.c:246)
>> ==10884==    by 0x40D4DE: ovsdb_jsonrpc_trigger_create (jsonrpc-server.c:1119)
>> ==10884==    by 0x40D4DE: ovsdb_jsonrpc_session_got_request (jsonrpc-server.c:986)
>> ==10884==    by 0x40D4DE: ovsdb_jsonrpc_session_run (jsonrpc-server.c:556)
>> ==10884==    by 0x40D4DE: ovsdb_jsonrpc_session_run_all (jsonrpc-server.c:586)
>> ==10884==    by 0x40D4DE: ovsdb_jsonrpc_server_run (jsonrpc-server.c:401)
>> ==10884==    by 0x406A6E: main_loop (ovsdb-server.c:209)
>> ==10884==    by 0x406A6E: main (ovsdb-server.c:460)
>>
>> 'new_schema' should also be freed when there is no error.
>> This patch fixes it.
>>
>> Signed-off-by: Yifeng Sun <pkusunyifeng at gmail.com>
>> ---
>>  ovsdb/trigger.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ovsdb/trigger.c b/ovsdb/trigger.c
>> index 6f4ed96b000b..7e62e90ae381 100644
>> --- a/ovsdb/trigger.c
>> +++ b/ovsdb/trigger.c
>> @@ -254,8 +254,8 @@ ovsdb_trigger_try(struct ovsdb_trigger *t, long long int now)
>>              if (!error) {
>>                  error = ovsdb_convert(t->db, new_schema, &newdb);
>>              }
>> +            ovsdb_schema_destroy(new_schema);
>>              if (error) {
>> -                ovsdb_schema_destroy(new_schema);
>>                  trigger_convert_error(t, error);
>>                  return false;
>>              }
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list