Re: [Qemu-devel] [PATCH v2] block/nbd: Fix the regression to free leaked visitor

[Ashijeet Acharya]

On Wed, Nov 2, 2016 at 4:00 PM, Kevin Wolf <address@hidden> wrote:
> Am 02.11.2016 um 10:48 hat Ashijeet Acharya geschrieben:
>> This patch frees the leaked visitor in nbd_refresh_filename() and uses
>> visit_free() to fix it. The leak was introduced by the commit 491d6c7.
>>
>> Signed-off-by: Ashijeet Acharya <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>
> I don't think this would generally be called a regression, so I'd change
> the subject to just "nbd: Fix leaker visitor".
>
>> Changes in v2:
>> - Include the regression commit id in the commit message
>> ---
>>  block/nbd.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index 8ef1438..ff9d01a 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -545,6 +545,7 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
>> QDict *options)
>>          qdict_put(opts, "tls-creds", qstring_from_str(s->tlscredsid));
>>      }
>>
>> +    visit_free(ov);
>>      qdict_flatten(opts);
>>      bs->full_open_options = opts;
>>  }
>
> I would prefer freeing the visitor immediately after visit_complete() so
> that everything visitor related is in a single place.
>
> Both of these points don't make your patch wrong, of course, but would
> you mind changing them?

Sure, I will send v3 straight away.

Ashijeet

> Kevin