Am 05.07.2018 um 18:04 hat Philippe Mathieu-Daudé geschrieben:
> 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.
Sorry, yes, you're right. I read 'if (ret < 0)' in your patch.
The reason for the seemingly superfluous error path is that you can add
new code behind it without having to modify the existing code.
Kevin
