| Message ID | 20200329041438.7815-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel] avutil/opt: Don't use NULL for %s string in a log message | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Quoting Andreas Rheinhardt (2020-03-29 06:14:37) > If one calls av_opt_set() with an incorrect string to set the value of > an option of type AV_OPT_TYPE_VIDEO_RATE, the given string is used in a > log message via %s. This also happens when the string is actually a > nullpointer in which case using it for %s is forbidden. > > This commit changes this by erroring out early in case of a nullpointer. > > This also fixes a warning from GCC 9.2: > "‘%s’ directive argument is null [-Wformat-overflow=]" > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- Looks ok
Anton Khirnov: > Quoting Andreas Rheinhardt (2020-03-29 06:14:37) >> If one calls av_opt_set() with an incorrect string to set the value of >> an option of type AV_OPT_TYPE_VIDEO_RATE, the given string is used in a >> log message via %s. This also happens when the string is actually a >> nullpointer in which case using it for %s is forbidden. >> >> This commit changes this by erroring out early in case of a nullpointer. >> >> This also fixes a warning from GCC 9.2: >> "‘%s’ directive argument is null [-Wformat-overflow=]" >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >> --- > > Looks ok > Applied, thanks. - Andreas
diff --git a/libavutil/opt.c b/libavutil/opt.c index a482febf5f..bf2562737b 100644 --- a/libavutil/opt.c +++ b/libavutil/opt.c @@ -330,12 +330,7 @@ static int set_string_image_size(void *obj, const AVOption *o, const char *val, static int set_string_video_rate(void *obj, const AVOption *o, const char *val, AVRational *dst) { - int ret; - if (!val) { - ret = AVERROR(EINVAL); - } else { - ret = av_parse_video_rate(dst, val); - } + int ret = av_parse_video_rate(dst, val); if (ret < 0) av_log(obj, AV_LOG_ERROR, "Unable to parse option value \"%s\" as video rate\n", val); return ret; @@ -473,7 +468,7 @@ int av_opt_set(void *obj, const char *name, const char *val, int search_flags) return AVERROR_OPTION_NOT_FOUND; if (!val && (o->type != AV_OPT_TYPE_STRING && o->type != AV_OPT_TYPE_PIXEL_FMT && o->type != AV_OPT_TYPE_SAMPLE_FMT && - o->type != AV_OPT_TYPE_IMAGE_SIZE && o->type != AV_OPT_TYPE_VIDEO_RATE && + o->type != AV_OPT_TYPE_IMAGE_SIZE && o->type != AV_OPT_TYPE_DURATION && o->type != AV_OPT_TYPE_COLOR && o->type != AV_OPT_TYPE_CHANNEL_LAYOUT && o->type != AV_OPT_TYPE_BOOL)) return AVERROR(EINVAL);
If one calls av_opt_set() with an incorrect string to set the value of an option of type AV_OPT_TYPE_VIDEO_RATE, the given string is used in a log message via %s. This also happens when the string is actually a nullpointer in which case using it for %s is forbidden. This commit changes this by erroring out early in case of a nullpointer. This also fixes a warning from GCC 9.2: "‘%s’ directive argument is null [-Wformat-overflow=]" Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- There are two places that call set_string_video_rate(): av_opt_set() and av_opt_set_defaults2(). This patch makes sure that av_opt_set() can't call set_string_video_rate() with a NULL string. All AV_OPT_TYPE_VIDEO_RATE options in the FFmpeg codebase have a default value different than NULL and so I deemed a check in the av_opt_set_defaults2() codepath to be unnecessary. But others might have a different opinion on this. libavutil/opt.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)