[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block/crypto: Simplify block_crypto_co_create_o
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH] block/crypto: Simplify block_crypto_co_create_opts_luks to avoid a memory leak |
Date: |
Thu, 5 Jul 2018 13:04:37 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 07/05/2018 07:20 AM, Kevin Wolf wrote:
> Am 04.07.2018 um 17:02 hat Philippe Mathieu-Daudé geschrieben:
>> After 1ec4f4160a1 Coverity reported:
>>
>> Variable cryptoopts going out of scope leaks the storage it points to.
>>
>> Fixes: Coverity CID 1393782 (Resource leak)
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>
> I already sent a much simpler fix:
> [PATCH] block/crypto: Fix memory leak in create error path
Oh OK I searched a bit but missed it.
> The only thing that is needed is replacing the return with a goto.
> Splitting it in three different error paths is unnecessary because the
> cleanup function handle NULL values just fine.
OK, good to know.
>> I think this check is superfluous but I respected the previous code:
>>
>> ret = block_crypto_co_create_generic(bs, size, create_opts, errp);
>> if (ret > 0) {
>> ret = 0;
>> }
>
> It is wrong, too. The old code keep the error code, goto fail skipped
> the ret = 0.
So this is not particularly wrong but as superfluous as the current use :)
ret = 0 is only useful if block_crypto_co_create_generic() returned a
value > 0, which seems unlikely.
Regards,
Phil.