diff mbox series

[FFmpeg-devel,2/5] avcodec/avcodec: Don't use NULL for %s printf specifier

Message ID 20210321094722.447294-2-andreas.rheinhardt@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel,1/5] avcodec/avcodec: Use dedicated pointer to access AVCodecInternal
Related show

Checks

Context Check Description
andriy/x86_make_warn warning New warnings during build
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt March 21, 2021, 9:47 a.m. UTC
Our "get name" functions can return NULL for invalid/unknown
arguments. So check for this.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
av_get_colorspace_name() can even return NULL for supported colorspaces.

 libavcodec/avcodec.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

Comments

Andreas Rheinhardt March 21, 2021, 8:42 p.m. UTC | #1
Andreas Rheinhardt:
> Our "get name" functions can return NULL for invalid/unknown
> arguments. So check for this.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> av_get_colorspace_name() can even return NULL for supported colorspaces.
> 
>  libavcodec/avcodec.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index 3088d2ff3f..93383dc9fb 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -607,6 +607,7 @@ void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
>      int new_line = 0;
>      AVRational display_aspect_ratio;
>      const char *separator = enc->dump_separator ? (const char *)enc->dump_separator : ", ";
> +    const char *str;
>  
>      if (!buf || buf_size <= 0)
>          return;
> @@ -642,14 +643,14 @@ void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
>              av_strlcat(buf, separator, buf_size);
>  
>              snprintf(buf + strlen(buf), buf_size - strlen(buf),
> -                 "%s", enc->pix_fmt == AV_PIX_FMT_NONE ? "none" :
> -                     av_get_pix_fmt_name(enc->pix_fmt));
> +                     "%s", enc->pix_fmt == AV_PIX_FMT_NONE ? "none" :
> +                     av_x_if_null(av_get_pix_fmt_name(enc->pix_fmt), "unknown"));
>              if (enc->bits_per_raw_sample && enc->pix_fmt != AV_PIX_FMT_NONE &&
>                  enc->bits_per_raw_sample < av_pix_fmt_desc_get(enc->pix_fmt)->comp[0].depth)
>                  av_strlcatf(detail, sizeof(detail), "%d bpc, ", enc->bits_per_raw_sample);
> -            if (enc->color_range != AVCOL_RANGE_UNSPECIFIED)
> -                av_strlcatf(detail, sizeof(detail), "%s, ",
> -                            av_color_range_name(enc->color_range));
> +            if (enc->color_range != AVCOL_RANGE_UNSPECIFIED &&
> +                (str = av_color_range_name(enc->color_range)))
> +                av_strlcatf(detail, sizeof(detail), "%s, ", str);
>  
>              if (enc->colorspace != AVCOL_SPC_UNSPECIFIED ||
>                  enc->color_primaries != AVCOL_PRI_UNSPECIFIED ||
> @@ -658,12 +659,11 @@ void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
>                      enc->colorspace != (int)enc->color_trc) {
>                      new_line = 1;
>                      av_strlcatf(detail, sizeof(detail), "%s/%s/%s, ",
> -                                av_color_space_name(enc->colorspace),
> -                                av_color_primaries_name(enc->color_primaries),
> -                                av_color_transfer_name(enc->color_trc));
> -                } else
> -                    av_strlcatf(detail, sizeof(detail), "%s, ",
> -                                av_get_colorspace_name(enc->colorspace));
> +                                av_x_if_null(av_color_space_name(enc->colorspace), "unknown"),
> +                                av_x_if_null(av_color_primaries_name(enc->color_primaries), "unknown"),
> +                                av_x_if_null(av_color_transfer_name(enc->color_trc), "unkown"));

av_x_if_null() returns a pointer to void and so the compiler gave me
warnings for the above lines, because it expected a pointer to char. Yet
I ignored them, because I use the -pedantic flag for compilations, so I
am used to getting warnings if the pointer corresponding to %p is
something else than a pointer to void. Turns out this is not only a
pedantic thing, so I replaced av_x_if_null() by a static function
unknown_if_null() here that is completely type-safe.
This also fixed the "unkown" typo above.

> +                 } else if (str = av_get_colorspace_name(enc->colorspace))
> +                       av_strlcatf(detail, sizeof(detail), "%s, ", str);
>              }
>  
>              if (enc->field_order != AV_FIELD_UNKNOWN) {
> @@ -681,9 +681,9 @@ void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
>              }
>  
>              if (av_log_get_level() >= AV_LOG_VERBOSE &&
> -                enc->chroma_sample_location != AVCHROMA_LOC_UNSPECIFIED)
> -                av_strlcatf(detail, sizeof(detail), "%s, ",
> -                            av_chroma_location_name(enc->chroma_sample_location));
> +                enc->chroma_sample_location != AVCHROMA_LOC_UNSPECIFIED &&
> +                (str = av_chroma_location_name(enc->chroma_sample_location)))
> +                av_strlcatf(detail, sizeof(detail), "%s, ", str);
>  
>              if (strlen(detail) > 1) {
>                  detail[strlen(detail) - 2] = 0;
> @@ -741,9 +741,10 @@ void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
>                       "%d Hz, ", enc->sample_rate);
>          }
>          av_get_channel_layout_string(buf + strlen(buf), buf_size - strlen(buf), enc->channels, enc->channel_layout);
> -        if (enc->sample_fmt != AV_SAMPLE_FMT_NONE) {
> +        if (enc->sample_fmt != AV_SAMPLE_FMT_NONE &&
> +            (str = av_get_sample_fmt_name(enc->sample_fmt))) {
>              snprintf(buf + strlen(buf), buf_size - strlen(buf),
> -                     ", %s", av_get_sample_fmt_name(enc->sample_fmt));
> +                     ", %s", str);
>          }
>          if (   enc->bits_per_raw_sample > 0
>              && enc->bits_per_raw_sample != av_get_bytes_per_sample(enc->sample_fmt) * 8)
>
diff mbox series

Patch

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 3088d2ff3f..93383dc9fb 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -607,6 +607,7 @@  void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
     int new_line = 0;
     AVRational display_aspect_ratio;
     const char *separator = enc->dump_separator ? (const char *)enc->dump_separator : ", ";
+    const char *str;
 
     if (!buf || buf_size <= 0)
         return;
@@ -642,14 +643,14 @@  void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
             av_strlcat(buf, separator, buf_size);
 
             snprintf(buf + strlen(buf), buf_size - strlen(buf),
-                 "%s", enc->pix_fmt == AV_PIX_FMT_NONE ? "none" :
-                     av_get_pix_fmt_name(enc->pix_fmt));
+                     "%s", enc->pix_fmt == AV_PIX_FMT_NONE ? "none" :
+                     av_x_if_null(av_get_pix_fmt_name(enc->pix_fmt), "unknown"));
             if (enc->bits_per_raw_sample && enc->pix_fmt != AV_PIX_FMT_NONE &&
                 enc->bits_per_raw_sample < av_pix_fmt_desc_get(enc->pix_fmt)->comp[0].depth)
                 av_strlcatf(detail, sizeof(detail), "%d bpc, ", enc->bits_per_raw_sample);
-            if (enc->color_range != AVCOL_RANGE_UNSPECIFIED)
-                av_strlcatf(detail, sizeof(detail), "%s, ",
-                            av_color_range_name(enc->color_range));
+            if (enc->color_range != AVCOL_RANGE_UNSPECIFIED &&
+                (str = av_color_range_name(enc->color_range)))
+                av_strlcatf(detail, sizeof(detail), "%s, ", str);
 
             if (enc->colorspace != AVCOL_SPC_UNSPECIFIED ||
                 enc->color_primaries != AVCOL_PRI_UNSPECIFIED ||
@@ -658,12 +659,11 @@  void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
                     enc->colorspace != (int)enc->color_trc) {
                     new_line = 1;
                     av_strlcatf(detail, sizeof(detail), "%s/%s/%s, ",
-                                av_color_space_name(enc->colorspace),
-                                av_color_primaries_name(enc->color_primaries),
-                                av_color_transfer_name(enc->color_trc));
-                } else
-                    av_strlcatf(detail, sizeof(detail), "%s, ",
-                                av_get_colorspace_name(enc->colorspace));
+                                av_x_if_null(av_color_space_name(enc->colorspace), "unknown"),
+                                av_x_if_null(av_color_primaries_name(enc->color_primaries), "unknown"),
+                                av_x_if_null(av_color_transfer_name(enc->color_trc), "unkown"));
+                 } else if (str = av_get_colorspace_name(enc->colorspace))
+                       av_strlcatf(detail, sizeof(detail), "%s, ", str);
             }
 
             if (enc->field_order != AV_FIELD_UNKNOWN) {
@@ -681,9 +681,9 @@  void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
             }
 
             if (av_log_get_level() >= AV_LOG_VERBOSE &&
-                enc->chroma_sample_location != AVCHROMA_LOC_UNSPECIFIED)
-                av_strlcatf(detail, sizeof(detail), "%s, ",
-                            av_chroma_location_name(enc->chroma_sample_location));
+                enc->chroma_sample_location != AVCHROMA_LOC_UNSPECIFIED &&
+                (str = av_chroma_location_name(enc->chroma_sample_location)))
+                av_strlcatf(detail, sizeof(detail), "%s, ", str);
 
             if (strlen(detail) > 1) {
                 detail[strlen(detail) - 2] = 0;
@@ -741,9 +741,10 @@  void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
                      "%d Hz, ", enc->sample_rate);
         }
         av_get_channel_layout_string(buf + strlen(buf), buf_size - strlen(buf), enc->channels, enc->channel_layout);
-        if (enc->sample_fmt != AV_SAMPLE_FMT_NONE) {
+        if (enc->sample_fmt != AV_SAMPLE_FMT_NONE &&
+            (str = av_get_sample_fmt_name(enc->sample_fmt))) {
             snprintf(buf + strlen(buf), buf_size - strlen(buf),
-                     ", %s", av_get_sample_fmt_name(enc->sample_fmt));
+                     ", %s", str);
         }
         if (   enc->bits_per_raw_sample > 0
             && enc->bits_per_raw_sample != av_get_bytes_per_sample(enc->sample_fmt) * 8)