[FFmpeg-devel] [PATCH v2 4/5] avcodec/mpegvideo: return more specific error codes for ff_mpv_common_init()

[lance.lmwang at gmail.com]

On Thu, May 07, 2020 at 08:34:44PM +0200, Marton Balint wrote:
> 
> 
> On Thu, 7 May 2020, lance.lmwang at gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang at gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> > ---
> > libavcodec/mpegvideo.c | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> > index 22cab28..b5ddb1b 100644
> > --- a/libavcodec/mpegvideo.c
> > +++ b/libavcodec/mpegvideo.c
> > @@ -889,7 +889,7 @@ static void clear_context(MpegEncContext *s)
> >  */
> > av_cold int ff_mpv_common_init(MpegEncContext *s)
> > {
> > -    int i, ret;
> > +    int i, ret = AVERROR(ENOMEM);
> 
> This initialization is unnecessary here, you will overwrite ret later.
> 
> >     int nb_slices = (HAVE_THREADS &&
> >                      s->avctx->active_thread_type & FF_THREAD_SLICE) ?
> >                     s->avctx->thread_count : 1;
> > @@ -907,7 +907,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
> >     if (s->avctx->pix_fmt == AV_PIX_FMT_NONE) {
> >         av_log(s->avctx, AV_LOG_ERROR,
> >                "decoding to AV_PIX_FMT_NONE is not supported.\n");
> > -        return -1;
> > +        return AVERROR(EINVAL);
> >     }
> > 
> >     if (nb_slices > MAX_THREADS || (nb_slices > s->mb_height && s->mb_height)) {
> > @@ -923,7 +923,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
> > 
> >     if ((s->width || s->height) &&
> >         av_image_check_size(s->width, s->height, 0, s->avctx))
> > -        return -1;
> > +        return AVERROR(EINVAL);
> > 
> >     dct_init(s);
> > 
> > @@ -954,7 +954,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
> >     if (!s->new_picture.f)
> >         goto fail;
> > 
> > -    if (init_context_frame(s))
> > +    if ((ret = init_context_frame(s)))
> >         goto fail;
> > 
> >     s->parse_context.state = -1;
> > @@ -971,7 +971,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
> >                 if (!s->thread_context[i])
> >                     goto fail;
> >             }
> > -            if (init_duplicate_context(s->thread_context[i]) < 0)
> > +            if ((ret = init_duplicate_context(s->thread_context[i])) < 0)
> >                 goto fail;
> >             s->thread_context[i]->start_mb_y =
> >                 (s->mb_height * (i) + nb_slices / 2) / nb_slices;
> > @@ -979,7 +979,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
> >                 (s->mb_height * (i + 1) + nb_slices / 2) / nb_slices;
> >         }
> >     } else {
> > -        if (init_duplicate_context(s) < 0)
> > +        if ((ret = init_duplicate_context(s)) < 0)
> >             goto fail;
> >         s->start_mb_y = 0;
> >         s->end_mb_y   = s->mb_height;
> > @@ -990,7 +990,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
> >     return 0;
> >  fail:
> >     ff_mpv_common_end(s);
> > -    return -1;
> > +    return ret
> 
> Not all goto fail set ret. I think it is cleaner if you add a new label
> "fail_nomem" before the fail label and set ret to ENOMEM there, and use that
> label for ENOMEM cases.

thanks for review, will update to add fail_nomem label.
> 
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang