On Tue, 12 May 2020, Nicolas George wrote:
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.
And you assume that I want to assign the error code to ret. Wrong. What if
I want to return it as is? Or what if I want to return NULL beacuse the
function returns a pointer? Using variables is complicated. Constants
make the code more simple and readable.
Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
