[FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[Nicolas George]

Marton Balint (12020-05-12):
> > -    FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->buffer.samples, s->channels, 3 * 1024 * sizeof(s->buffer.samples[0]), alloc_fail);
> > -    FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->cpe, s->chan_map[0], sizeof(ChannelElement), alloc_fail);
> > +    int ret, ch;
> > +    FF_ALLOCZ_ARRAY_OR_GOTO(s->buffer.samples, s->channels, 3 * 1024 * sizeof(s->buffer.samples[0]), ret, return ret);
> > +    FF_ALLOCZ_ARRAY_OR_GOTO(s->cpe, s->chan_map[0], sizeof(ChannelElement), ret, return ret);
> 
> If you want to change the existing macro, then you have to rename it,
> because it no longer does goto...

Indeed. But also, it must do the same. Replacing a goto with a direct
return is not correct.

> Also I kind of disagree with Nicholas that we should always assign a ret
> variable, I think a single error statement is better. For example in the
> case above a goto fail is a lot nicer (and returning ENOMEM) there.

This is a mistake because you invent the error code on the call site.
The function/macro knows what the error code, the caller uses the error
code. Otherwise, changes in the function/macro can leave the error code
invalid.

We should never let the caller assume the error code.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200512/a1400be6/attachment.sig>