diff mbox series

[FFmpeg-devel] avutil/opt: Don't use NULL for %s string in a log message

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
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt March 29, 2020, 4:14 a.m. UTC
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(-)

Comments

Anton Khirnov March 31, 2020, 9:53 a.m. UTC | #1
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
Andreas Rheinhardt April 1, 2020, 2:13 a.m. UTC | #2
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 mbox series

Patch

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);