| Message ID | 20200328001733.695-2-lance.lmwang@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,v2,1/3] avfilter/af_acrossover: Check sscanf() return value | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
lance.lmwang@gmail.com (12020-03-28): > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > libavfilter/vf_mix.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Why? What do you expect to change with this patch, and how is it supposed to be better? Regards,
On Sat, Mar 28, 2020 at 08:10:51PM +0100, Nicolas George wrote: > lance.lmwang@gmail.com (12020-03-28): > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > libavfilter/vf_mix.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > Why? What do you expect to change with this patch, and how is it > supposed to be better? If av_sscanf failed, s->weights[i] is invalid data and can't be used further. As the old code allow to use the last weights, so I use continue to avoid invalid access only. Do you think it's better to print out one warning message? > > Regards, > > -- > Nicolas George
Limin Wang (12020-03-29): > If av_sscanf failed, s->weights[i] is invalid data and can't be used > further. I think you are mistaken: if av_sscanf() fails, s->weights[i] stays unchanged: it was 0, it's still 0. > As the old code allow to use the last weights, so I use > continue to avoid invalid access only. It's not invalid, it's 0. > Do you think it's better to > print out one warning message? With this correction, I think this change is not necessary at all. Regards,
On Sun, Mar 29, 2020 at 12:31:40AM +0100, Nicolas George wrote: > Limin Wang (12020-03-29): > > If av_sscanf failed, s->weights[i] is invalid data and can't be used > > further. > > I think you are mistaken: if av_sscanf() fails, s->weights[i] stays > unchanged: it was 0, it's still 0. > > > As the old code allow to use the last weights, so I use > > continue to avoid invalid access only. > > It's not invalid, it's 0. > > > Do you think it's better to > > print out one warning message? > > With this correction, I think this change is not necessary at all. Yes, the result is same with my change. Anyway if the user option is typo, the result may be unexpected for user. > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Limin Wang (12020-03-29): > Anyway if the user option is typo, > the result may be unexpected for user. But your change does not address that. For that, you would print a message and return an error. Which is probably the right thing to do, actually. Regards,
On Sun, Mar 29, 2020 at 01:32:59AM +0100, Nicolas George wrote: > Limin Wang (12020-03-29): > > Anyway if the user option is typo, > > the result may be unexpected for user. > > But your change does not address that. For that, you would print a > message and return an error. Which is probably the right thing to do, > actually. OK, update the patch to print a message and return error. > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Limin Wang (12020-03-29):
> OK, update the patch to print a message and return error.
Thanks. I have no objection to that new version, but I do not maintain
that code.
Regards,
On Sun, Mar 29, 2020 at 11:18:11AM +0200, Nicolas George wrote: > Limin Wang (12020-03-29): > > OK, update the patch to print a message and return error. > > Thanks. I have no objection to that new version, but I do not maintain > that code. ping for the patchset for merge. > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavfilter/vf_mix.c b/libavfilter/vf_mix.c index 9e1ae79..e47f7e2 100644 --- a/libavfilter/vf_mix.c +++ b/libavfilter/vf_mix.c @@ -108,7 +108,8 @@ static av_cold int init(AVFilterContext *ctx) break; p = NULL; - av_sscanf(arg, "%f", &s->weights[i]); + if (av_sscanf(arg, "%f", &s->weights[i]) != 1) + continue; s->wfactor += s->weights[i]; last = i; }