On Thu, Feb 19, 2015 at 12:04:01PM -0500, Vittorio Giovara wrote:
> --- a/libavcodec/asvenc.c
> +++ b/libavcodec/asvenc.c
> @@ -298,6 +298,8 @@ static av_cold int encode_init(AVCodecContext *avctx)
> avctx->global_quality / 2) / avctx->global_quality;
>
> avctx->extradata = av_mallocz(8);
> + if (!avctx->extradata)
> + return AVERROR(ENOMEM);
What happens with avctx->coded_frame allocated above?
> --- a/libavcodec/eatgv.c
> +++ b/libavcodec/eatgv.c
> @@ -174,6 +174,8 @@ static int tgv_decode_inter(TgvContext *s, AVFrame *frame,
> /* allocate codebook buffers as necessary */
> if (num_mvs > s->num_mvs) {
> s->mv_codebook = av_realloc(s->mv_codebook, num_mvs*2*sizeof(int));
> + if (!s->mv_codebook)
> + return AVERROR(ENOMEM);
Looks like this is leaked on av_reallocp() failure below.
> --- a/libavcodec/ffv1.c
> +++ b/libavcodec/ffv1.c
> @@ -203,6 +203,8 @@ av_cold int ffv1_init_slice_contexts(FFV1Context *f)
> int sxe = f->avctx->width * (sx + 1) / f->num_h_slices;
> int sys = f->avctx->height * sy / f->num_v_slices;
> int sye = f->avctx->height * (sy + 1) / f->num_v_slices;
> + if (!fs)
> + return AVERROR(ENOMEM);
This is leaked on a subsequent malloc failure below.
> --- a/libavcodec/huffyuvenc.c
> +++ b/libavcodec/huffyuvenc.c
> @@ -154,7 +154,7 @@ static av_cold int encode_init(AVCodecContext *avctx)
> avctx->coded_frame = av_frame_alloc();
> - if (!avctx->coded_frame)
> + if (!avctx->extradata || !avctx->stats_out || !avctx->coded_frame)
> return AVERROR(ENOMEM);
This leaks memory if some but not all allocations succeed.
> --- a/libavcodec/jpeglsdec.c
> +++ b/libavcodec/jpeglsdec.c
> @@ -277,10 +277,18 @@ int ff_jpegls_decode_picture(MJpegDecodeContext *s, int
> near,
>
> zero = av_mallocz(s->picture_ptr->linesize[0]);
> + if (!zero) {
> + ret = AVERROR(ENOMEM);
> + goto end;
> + }
> last = zero;
> cur = s->picture_ptr->data[0];
>
> state = av_mallocz(sizeof(JLSState));
> + if (!state) {
> + ret = AVERROR(ENOMEM);
> + goto end;
> + }
"goto end" is overkill here. Just return in the first case. In the second
you could free and return or keep the goto.
> --- a/libavcodec/jpeglsenc.c
> +++ b/libavcodec/jpeglsenc.c
> @@ -270,6 +270,8 @@ static int encode_picture_ls(AVCodecContext *avctx,
> AVPacket *pkt,
>
> buf2 = av_malloc(pkt->size);
> + if (!buf2)
> + return AVERROR(ENOMEM);
>
> @@ -300,6 +302,8 @@ static int encode_picture_ls(AVCodecContext *avctx,
> AVPacket *pkt,
>
> state = av_mallocz(sizeof(JLSState));
> + if (!state)
> + return AVERROR(ENOMEM);
> /* initialize JPEG-LS state from JPEG parameters */
> @@ -309,6 +313,8 @@ static int encode_picture_ls(AVCodecContext *avctx,
> AVPacket *pkt,
> zero = av_mallocz(p->linesize[0]);
> + if (!zero)
> + return AVERROR(ENOMEM);
> last = zero;
more memleaks
> --- a/libavcodec/lclenc.c
> +++ b/libavcodec/lclenc.c
> @@ -134,6 +134,8 @@ static av_cold int encode_init(AVCodecContext *avctx)
> assert(avctx->width && avctx->height);
>
> avctx->extradata= av_mallocz(8);
> + if (!avctx->extradata)
> + return AVERROR(ENOMEM);
>
> avctx->coded_frame = av_frame_alloc();
> if (!avctx->coded_frame)
What happens if the first allocation succeeds but the second fails?
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -531,6 +531,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
>
> s = x264_encoder_headers(x4->enc, &nal, &nnal);
> avctx->extradata = p = av_malloc(s);
> + if (!p)
> + return AVERROR(ENOMEM);
>
> for (i = 0; i < nnal; i++) {
> /* Don't put the SEI in extradata. */
> @@ -538,6 +540,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
> av_log(avctx, AV_LOG_INFO, "%s\n", nal[i].p_payload+25);
> x4->sei_size = nal[i].i_payload;
> x4->sei = av_malloc(x4->sei_size);
> + if (!x4->sei)
> + return AVERROR(ENOMEM);
> memcpy(x4->sei, nal[i].p_payload, nal[i].i_payload);
> continue;
Returning w/o free will leak memory.
> --- a/libavcodec/libxvid.c
> +++ b/libavcodec/libxvid.c
> @@ -593,11 +595,15 @@ static av_cold int xvid_encode_init(AVCodecContext
> *avctx)
> if (avctx->intra_matrix) {
> intra = avctx->intra_matrix;
> x->intra_matrix = av_malloc(sizeof(unsigned char) * 64);
> + if (!x->intra_matrix)
> + return AVERROR(ENOMEM);
> } else
> intra = NULL;
> if (avctx->inter_matrix) {
> inter = avctx->inter_matrix;
> x->inter_matrix = av_malloc(sizeof(unsigned char) * 64);
> + if (!x->inter_matrix)
> + return AVERROR(ENOMEM);
> } else
> inter = NULL;
I think you need to clean up in the second case.
> --- a/libavcodec/psymodel.c
> +++ b/libavcodec/psymodel.c
> @@ -98,6 +104,8 @@ av_cold struct FFPsyPreprocessContext*
> ff_psy_preprocess_init(AVCodecContext *av
> float cutoff_coeff = 0;
> ctx = av_mallocz(sizeof(FFPsyPreprocessContext));
> + if (!ctx)
> + return NULL;
> ctx->avctx = avctx;
>
> @@ -109,6 +117,10 @@ av_cold struct FFPsyPreprocessContext*
> ff_psy_preprocess_init(AVCodecContext *av
> if (ctx->fcoeffs) {
> ctx->fstate = av_mallocz(sizeof(ctx->fstate[0]) * avctx->channels);
> + if (!ctx->fstate) {
> + av_freep(&ctx);
> + return NULL;
> + }
Here you do clean up properly :)
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -584,8 +584,13 @@ int ff_frame_thread_init(AVCodecContext *avctx)
> }
>
> avctx->internal->thread_ctx = fctx =
> av_mallocz(sizeof(FrameThreadContext));
> + if (!fctx)
> + return AVERROR(ENOMEM);
>
> fctx->threads = av_mallocz(sizeof(PerThreadContext) * thread_count);
> + if (!fctx->threads)
> + return AVERROR(ENOMEM);
Here you don't :(
> --- a/libavcodec/ratecontrol.c
> +++ b/libavcodec/ratecontrol.c
> @@ -931,6 +931,11 @@ static int init_pass2(MpegEncContext *s)
>
> qscale = av_malloc(sizeof(double) * rcc->num_entries);
> blurred_qscale = av_malloc(sizeof(double) * rcc->num_entries);
> + if (!qscale || !blurred_qscale) {
> + av_freep(&qscale);
> + av_freep(&blurred_qscale);
> + return AVERROR(ENOMEM);
> + }
These are local variables, setting them to NULL before returning is pointless
overkill.
There's an av_malloc() in that file that you leave unchecked.
> --- a/libavcodec/svq1enc.c
> +++ b/libavcodec/svq1enc.c
> @@ -293,6 +293,8 @@ static int svq1_encode_plane(SVQ1EncContext *s, int plane,
> s->motion_val16[plane] = av_mallocz((s->m.mb_stride *
> (block_height + 2) + 1) *
> 2 * sizeof(int16_t));
> + if (!s->motion_val8[plane] || !s->motion_val16[plane])
> + return AVERROR(ENOMEM);
> }
.. memleak I fear ..
> @@ -193,10 +197,15 @@ static int tm2_build_huff_table(TM2Context *ctx,
> TM2Codes *code)
> code->bits = huff.max_bits;
> code->length = huff.max_num;
> code->recode = av_malloc(code->length * sizeof(int));
> + if (!code->recode) {
> + res = AVERROR(ENOMEM);
> + goto out;
> + }
> for (i = 0; i < code->length; i++)
> code->recode[i] = huff.nums[i];
> }
> }
> +out:
> /* free allocated memory */
> av_free(huff.nums);
> av_free(huff.bits);
I suggest an empty line before the goto label.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
