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.
-- 
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to