On 2011-12-09 14:18:03 +0000, Måns Rullgård wrote:
> Janne Grunau <[email protected]> writes:
> 
> >> > +int ff_rv34_decode_init_thread_copy(AVCodecContext *avctx)
> >> > +{
> >> > +    RV34DecContext *r = avctx->priv_data;
> >> > +
> >> > +    r->s.avctx = avctx;
> >> > +
> >> > +    if (avctx->internal->is_copy) {
> >> > +        FF_ALLOC_OR_GOTO(avctx, r->intra_types_hist, 
> >> > r->intra_types_stride *
> >> > +                         4 * 2 * sizeof(*r->intra_types_hist),       
> >> > err_mem1);
> >> > +        FF_ALLOC_OR_GOTO(avctx, r->mb_type,          r->s.mb_stride *
> >> > +                         r->s.mb_height * sizeof(*r->mb_type),       
> >> > err_mem2);
> >> > +        FF_ALLOC_OR_GOTO(avctx, r->cbp_luma,         r->s.mb_stride *
> >> > +                         r->s.mb_height * sizeof(*r->cbp_luma),      
> >> > err_mem3);
> >> > +        FF_ALLOC_OR_GOTO(avctx, r->cbp_chroma,       r->s.mb_stride *
> >> > +                         r->s.mb_height * sizeof(*r->cbp_chroma),    
> >> > err_mem4);
> >> > +        FF_ALLOC_OR_GOTO(avctx, r->deblock_coefs,    r->s.mb_stride *
> >> > +                         r->s.mb_height * sizeof(*r->deblock_coefs), 
> >> > err_mem5);
> >> > +
> >> > +        r->intra_types      = r->intra_types_hist + 
> >> > r->intra_types_stride * 4;
> >> > +        r->tmp_b_block_base = NULL;
> >> > +
> >> > +        memset(r->mb_type, 0,  r->s.mb_stride * r->s.mb_height *
> >> > +               sizeof(*r->mb_type));
> >> > +
> >> > +        MPV_common_init(&r->s);
> >> > +    }
> >> > +    return 0;
> >> > +err_mem5:
> >> > +    av_freep(&r->cbp_chroma);
> >> > +err_mem4:
> >> > +    av_freep(&r->cbp_luma);
> >> > +err_mem3:
> >> > +    av_freep(&r->mb_type);
> >> > +err_mem2:
> >> > +    av_freep(&r->intra_types_hist);
> >> > +    r->intra_types = NULL;
> >> > +err_mem1:
> >> > +    return AVERROR(ENOMEM);
> >> 
> >> No need for separate labels.  Freeing a null pointer is perfectly safe.
> >
> > The pointers are valid pointers copied from the first initialized codec.
> > I could set them explicitly to NULL first,
> 
> I see.  Another option is to simply av_malloc and assign them all
> normally, then do a single if (!ptr1 || !ptr2 ...) { free all; }
> That might look nicer in this simple context.

It avoids at least the ugly gotos. Change locally.

Janne 
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to