On Thu, Apr 9, 2015 at 4:40 PM, Vittorio Giovara
<[email protected]> wrote:
> On Thu, Apr 9, 2015 at 12:00 PM, Martin Storsjö <[email protected]> wrote:
>> On Thu, 9 Apr 2015, Martin Storsjö wrote:
>>
>>> On Thu, 9 Apr 2015, Vittorio Giovara wrote:
>>>
>>>> ---
>>>> libavcodec/libx264.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
>>>> index 8e19be4..a8c75fc 100644
>>>> --- a/libavcodec/libx264.c
>>>> +++ b/libavcodec/libx264.c
>>>> @@ -272,7 +272,7 @@ static av_cold int X264_close(AVCodecContext *avctx)
>>>>     X264Context *x4 = avctx->priv_data;
>>>>
>>>>     av_freep(&avctx->extradata);
>>>> -    av_free(x4->sei);
>>>> +    av_freep(&x4->sei);
>>>>
>>>>     if (x4->enc)
>>>>         x264_encoder_close(x4->enc);
>>>
>>>
>>> Note that this doesn't set x4->enc to NULL after freeing it. This is an
>>> indication that you could either have a double free if this would be NULL
>>> and init already cleans it up (and if init doesn't set it to NULL either),
>>> or that the init function currently leaks if it fails.
>>>
>>> In this case, it seems like the init function indeed actually leaks on
>>> error. Then you should mention that such leaks are fixed by this change as
>>> well, in the commit message, to make a clear distinction from changes which
>>> are pure refactorings.
>>
>>
>> Also, since this commit is more than just adding the flags (it also changes
>> code), I'd rather label it "Make it use the init-cleanup flag" or something
>> such in the commit message, to make the distinction that it isn't signaling
>> behaviour that was supported before, but refactoring things in order to be
>> able to set that flag.
>>
>
> Ok, I'll do everything and resend.

I've double checked all the set, and there was only a leak I missed in libxvid.
I've sent the amended version of the patches for libxvid and libx264,
while the rest of the set doesn't need further changes.
-- 
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to