| Message ID | 20171003045544.6632-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 3 Oct 2017 01:55:44 -0300 James Almer <jamrial@gmail.com> wrote: > Fixes memleaks introduced by skipping cleanup at the end of the > functions. > > Regression since a22c6a4796ca1f2cbee6784262515da876fbec22. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/encode.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/libavcodec/encode.c b/libavcodec/encode.c > index c152228c92..841a185738 100644 > --- a/libavcodec/encode.c > +++ b/libavcodec/encode.c > @@ -225,10 +225,10 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx, > } else if (!avpkt->buf) { > AVPacket tmp = { 0 }; > ret = av_packet_ref(&tmp, avpkt); > - if (ret < 0) > - return ret; > - av_packet_unref(avpkt); > - *avpkt = tmp; > + if (!ret) { > + av_packet_unref(avpkt); > + *avpkt = tmp; > + } > } > } > > @@ -324,10 +324,10 @@ int attribute_align_arg avcodec_encode_video2(AVCodecContext *avctx, > } else if (!avpkt->buf) { > AVPacket tmp = { 0 }; > ret = av_packet_ref(&tmp, avpkt); > - if (ret < 0) > - return ret; > - av_packet_unref(avpkt); > - *avpkt = tmp; > + if (!ret) { > + av_packet_unref(avpkt); > + *avpkt = tmp; > + } > } > } > Seems ok if ret is actually checked in the code below. Something like "goto cleanup;" on error would be less confusing though. Also why did you essentially change "!(ret<0)" to "!ret"? In theory, all positive return values indicate success.
On 10/3/2017 5:23 AM, wm4 wrote: > On Tue, 3 Oct 2017 01:55:44 -0300 > James Almer <jamrial@gmail.com> wrote: > >> Fixes memleaks introduced by skipping cleanup at the end of the >> functions. >> >> Regression since a22c6a4796ca1f2cbee6784262515da876fbec22. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/encode.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/libavcodec/encode.c b/libavcodec/encode.c >> index c152228c92..841a185738 100644 >> --- a/libavcodec/encode.c >> +++ b/libavcodec/encode.c >> @@ -225,10 +225,10 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx, >> } else if (!avpkt->buf) { >> AVPacket tmp = { 0 }; >> ret = av_packet_ref(&tmp, avpkt); >> - if (ret < 0) >> - return ret; >> - av_packet_unref(avpkt); >> - *avpkt = tmp; >> + if (!ret) { >> + av_packet_unref(avpkt); >> + *avpkt = tmp; >> + } >> } >> } >> >> @@ -324,10 +324,10 @@ int attribute_align_arg avcodec_encode_video2(AVCodecContext *avctx, >> } else if (!avpkt->buf) { >> AVPacket tmp = { 0 }; >> ret = av_packet_ref(&tmp, avpkt); >> - if (ret < 0) >> - return ret; >> - av_packet_unref(avpkt); >> - *avpkt = tmp; >> + if (!ret) { >> + av_packet_unref(avpkt); >> + *avpkt = tmp; >> + } >> } >> } >> > > Seems ok if ret is actually checked in the code below. Something like > "goto cleanup;" on error would be less confusing though. Did that instead and pushed. Thanks.
diff --git a/libavcodec/encode.c b/libavcodec/encode.c index c152228c92..841a185738 100644 --- a/libavcodec/encode.c +++ b/libavcodec/encode.c @@ -225,10 +225,10 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx, } else if (!avpkt->buf) { AVPacket tmp = { 0 }; ret = av_packet_ref(&tmp, avpkt); - if (ret < 0) - return ret; - av_packet_unref(avpkt); - *avpkt = tmp; + if (!ret) { + av_packet_unref(avpkt); + *avpkt = tmp; + } } } @@ -324,10 +324,10 @@ int attribute_align_arg avcodec_encode_video2(AVCodecContext *avctx, } else if (!avpkt->buf) { AVPacket tmp = { 0 }; ret = av_packet_ref(&tmp, avpkt); - if (ret < 0) - return ret; - av_packet_unref(avpkt); - *avpkt = tmp; + if (!ret) { + av_packet_unref(avpkt); + *avpkt = tmp; + } } }
Fixes memleaks introduced by skipping cleanup at the end of the functions. Regression since a22c6a4796ca1f2cbee6784262515da876fbec22. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/encode.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)