| Message ID | 20201013010701.53885-1-chris@miceli.net.au |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel] libavfilter/af_mcompand: Check for failure to allocate memory | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | warning | Make failed |
Chris Miceli: > ticket 8931 mentions some missing null pointer checks and so this is one ^ Nit: null pointer checks can be more than just checking allocations for success. So how about "This commit fixes some of the unchecked allocations mentioned in ticket #8931." > diff of a few which are for these fixes. There are previous code pieces > which return ENOMEM, so this should work as intended. Better check: You can e.g. add av_max_alloc(1); before the allocations. > --- > libavfilter/af_mcompand.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/libavfilter/af_mcompand.c b/libavfilter/af_mcompand.c > index f142573bea..953f7cd407 100644 > --- a/libavfilter/af_mcompand.c > +++ b/libavfilter/af_mcompand.c > @@ -386,8 +386,20 @@ static int config_output(AVFilterLink *outlink) > } > > s->bands[i].attack_rate = av_calloc(outlink->channels, sizeof(double)); > + if (!s->bands[i].attack_rate) { > + av_log(ctx, AV_LOG_ERROR, "Could not allocate memory for the attack_rate\n"); > + return AVERROR(ENOMEM); > + } > s->bands[i].decay_rate = av_calloc(outlink->channels, sizeof(double)); > + if (!s->bands[i].decay_rate) { > + av_log(ctx, AV_LOG_ERROR, "Could not allocate memory for the decay_rate\n"); > + return AVERROR(ENOMEM); > + } > s->bands[i].volume = av_calloc(outlink->channels, sizeof(double)); > + if (!s->bands[i].volume) { > + av_log(ctx, AV_LOG_ERROR, "Could not allocate memory for the volume\n"); > + return AVERROR(ENOMEM); > + } > for (k = 0; k < FFMIN(nb_attacks / 2, outlink->channels); k++) { > char *tstr3 = av_strtok(p3, ",", &saveptr3); > > I don't think it is necessary to add a log message for any of these: It is very unlikely that these errors will be triggered at all and if so, these messages won't help the user at all (well, they could look up in the source code what the array that couldn't be allocated is exactly, but why would they do this?). They just take up space in the binary and make the code less readable. - Andreas
diff of a few which are for these fixes. There are previous code pieces which return ENOMEM, so this should work as intended. --- libavfilter/af_mcompand.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libavfilter/af_mcompand.c b/libavfilter/af_mcompand.c index f142573bea..953f7cd407 100644 --- a/libavfilter/af_mcompand.c +++ b/libavfilter/af_mcompand.c @@ -386,8 +386,20 @@ static int config_output(AVFilterLink *outlink) } s->bands[i].attack_rate = av_calloc(outlink->channels, sizeof(double)); + if (!s->bands[i].attack_rate) { + av_log(ctx, AV_LOG_ERROR, "Could not allocate memory for the attack_rate\n"); + return AVERROR(ENOMEM); + } s->bands[i].decay_rate = av_calloc(outlink->channels, sizeof(double)); + if (!s->bands[i].decay_rate) { + av_log(ctx, AV_LOG_ERROR, "Could not allocate memory for the decay_rate\n"); + return AVERROR(ENOMEM); + } s->bands[i].volume = av_calloc(outlink->channels, sizeof(double)); + if (!s->bands[i].volume) { + av_log(ctx, AV_LOG_ERROR, "Could not allocate memory for the volume\n"); + return AVERROR(ENOMEM); + } for (k = 0; k < FFMIN(nb_attacks / 2, outlink->channels); k++) { char *tstr3 = av_strtok(p3, ",", &saveptr3);