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é <f4...@amsat.org> > > 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.