diff mbox series

[FFmpeg-devel,2/5] avcodec/packet: Also change av_packet_pack/unpack_dictionary to size_t

Message ID 20210317235958.1308987-2-andreas.rheinhardt@gmail.com
State Accepted
Commit a46d7819051e0e8c61017e75a0389389ae810ca4
Headers show
Series [FFmpeg-devel,1/5] avcodec/avpacket: Improve overflow checks when packing dictionary
Related show

Checks

Context Check Description
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 17, 2021, 11:59 p.m. UTC
These are auxiliary side-data functions, so they should have been
switched to size_t in d79e0fe65c51491f9bf8a470bbe36fb09f3e1280,
but this has been forgotten.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/avpacket.c        | 13 +++++++++++++
 libavcodec/packet.h          | 10 +++++++++-
 libavdevice/decklink_dec.cpp |  2 +-
 libavdevice/lavfi.c          |  2 +-
 libavformat/concatdec.c      |  2 +-
 libavformat/img2dec.c        |  3 ++-
 libavformat/oggdec.h         |  2 +-
 7 files changed, 28 insertions(+), 6 deletions(-)

Comments

James Almer March 18, 2021, 1:30 a.m. UTC | #1
On 3/17/2021 8:59 PM, Andreas Rheinhardt wrote:
> These are auxiliary side-data functions, so they should have been
> switched to size_t in d79e0fe65c51491f9bf8a470bbe36fb09f3e1280,
> but this has been forgotten.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>   libavcodec/avpacket.c        | 13 +++++++++++++
>   libavcodec/packet.h          | 10 +++++++++-
>   libavdevice/decklink_dec.cpp |  2 +-
>   libavdevice/lavfi.c          |  2 +-
>   libavformat/concatdec.c      |  2 +-
>   libavformat/img2dec.c        |  3 ++-
>   libavformat/oggdec.h         |  2 +-
>   7 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 8f0850fb00..b5bac5c5f2 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -507,7 +507,11 @@ int av_packet_split_side_data(AVPacket *pkt){
>   }
>   #endif
>   
> +#if FF_API_BUFFER_SIZE_T
>   uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size)
> +#else
> +uint8_t *av_packet_pack_dictionary(AVDictionary *dict, size_t *size)
> +#endif
>   {
>       uint8_t *data = NULL;
>       *size = 0;
> @@ -526,7 +530,11 @@ uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size)
>   
>                   if (pass)
>                       memcpy(data + total_length, str, len);
> +#if FF_API_BUFFER_SIZE_T
>                   else if (len > INT_MAX - total_length)
> +#else
> +                else if (len > SIZE_MAX - total_length)
> +#endif
>                       return NULL;
>                   total_length += len;
>               }
> @@ -542,7 +550,12 @@ uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size)
>       return data;
>   }
>   
> +#if FF_API_BUFFER_SIZE_T
>   int av_packet_unpack_dictionary(const uint8_t *data, int size, AVDictionary **dict)
> +#else
> +int av_packet_unpack_dictionary(const uint8_t *data, size_t size,
> +                                AVDictionary **dict)
> +#endif

You can use buffer_size_t to reduce the ifdeffery in both cases.

>   {
>       const uint8_t *end;
>       int ret;
> diff --git a/libavcodec/packet.h b/libavcodec/packet.h
> index da4377e09f..ca18ae631f 100644
> --- a/libavcodec/packet.h
> +++ b/libavcodec/packet.h
> @@ -647,7 +647,11 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type);
>    * @param size pointer to store the size of the returned data
>    * @return pointer to data if successful, NULL otherwise
>    */
> +#if FF_API_BUFFER_SIZE_T
>   uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size);
> +#else
> +uint8_t *av_packet_pack_dictionary(AVDictionary *dict, size_t *size);
> +#endif
>   /**
>    * Unpack a dictionary from side_data.
>    *
> @@ -656,8 +660,12 @@ uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size);
>    * @param dict the metadata storage dictionary
>    * @return 0 on success, < 0 on failure
>    */
> +#if FF_API_BUFFER_SIZE_T
>   int av_packet_unpack_dictionary(const uint8_t *data, int size, AVDictionary **dict);
> -
> +#else
> +int av_packet_unpack_dictionary(const uint8_t *data, size_t size,
> +                                AVDictionary **dict);
> +#endif
>   
>   /**
>    * Convenience function to free all the side data stored.
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 40c3dae968..79d96cd620 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -922,7 +922,6 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>                       const char *tc = av_timecode_make_string(&tcr, tcstr, 0);
>                       if (tc) {
>                           AVDictionary* metadata_dict = NULL;
> -                        int metadata_len;
>                           uint8_t* packed_metadata;
>   
>                           if (av_cmp_q(ctx->video_st->r_frame_rate, av_make_q(60, 1)) < 1) {
> @@ -937,6 +936,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>                           }
>   
>                           if (av_dict_set(&metadata_dict, "timecode", tc, 0) >= 0) {
> +                            buffer_size_t metadata_len;
>                               packed_metadata = av_packet_pack_dictionary(metadata_dict, &metadata_len);
>                               av_dict_free(&metadata_dict);
>                               if (packed_metadata) {
> diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
> index 94ad03268a..fdadff3f7f 100644
> --- a/libavdevice/lavfi.c
> +++ b/libavdevice/lavfi.c
> @@ -446,7 +446,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
>   
>       frame_metadata = frame->metadata;
>       if (frame_metadata) {
> -        int size;
> +        buffer_size_t size;
>           uint8_t *metadata = av_packet_pack_dictionary(frame_metadata, &size);
>   
>           if (!metadata) {
> diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
> index 6d5b9914f9..32d4a99010 100644
> --- a/libavformat/concatdec.c
> +++ b/libavformat/concatdec.c
> @@ -626,7 +626,7 @@ static int concat_read_packet(AVFormatContext *avf, AVPacket *pkt)
>              av_ts2str(pkt->pts), av_ts2timestr(pkt->pts, &st->time_base),
>              av_ts2str(pkt->dts), av_ts2timestr(pkt->dts, &st->time_base));
>       if (cat->cur_file->metadata) {
> -        int metadata_len;
> +        buffer_size_t metadata_len;
>           char* packed_metadata = av_packet_pack_dictionary(cat->cur_file->metadata, &metadata_len);
>           if (!packed_metadata)
>               return AVERROR(ENOMEM);
> diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
> index 6c78dada8c..be7149bb2f 100644
> --- a/libavformat/img2dec.c
> +++ b/libavformat/img2dec.c
> @@ -381,9 +381,10 @@ int ff_img_read_header(AVFormatContext *s1)
>    * as a dictionary, so it can be used by filters like 'drawtext'.
>    */
>   static int add_filename_as_pkt_side_data(char *filename, AVPacket *pkt) {
> -    int metadata_len, ret;
>       AVDictionary *d = NULL;
>       char *packed_metadata = NULL;
> +    buffer_size_t metadata_len;
> +    int ret;
>   
>       av_dict_set(&d, "lavf.image2dec.source_path", filename, 0);
>       av_dict_set(&d, "lavf.image2dec.source_basename", av_basename(filename), 0);
> diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h
> index 629a1d6262..bf982bfe1c 100644
> --- a/libavformat/oggdec.h
> +++ b/libavformat/oggdec.h
> @@ -87,7 +87,7 @@ struct ogg_stream {
>       int start_trimming; ///< set the number of packets to drop from the start
>       int end_trimming; ///< set the number of packets to drop from the end
>       uint8_t *new_metadata;
> -    unsigned int new_metadata_size;
> +    buffer_size_t new_metadata_size;
>       void *private;
>   };

LGTM either way.
diff mbox series

Patch

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 8f0850fb00..b5bac5c5f2 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -507,7 +507,11 @@  int av_packet_split_side_data(AVPacket *pkt){
 }
 #endif
 
+#if FF_API_BUFFER_SIZE_T
 uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size)
+#else
+uint8_t *av_packet_pack_dictionary(AVDictionary *dict, size_t *size)
+#endif
 {
     uint8_t *data = NULL;
     *size = 0;
@@ -526,7 +530,11 @@  uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size)
 
                 if (pass)
                     memcpy(data + total_length, str, len);
+#if FF_API_BUFFER_SIZE_T
                 else if (len > INT_MAX - total_length)
+#else
+                else if (len > SIZE_MAX - total_length)
+#endif
                     return NULL;
                 total_length += len;
             }
@@ -542,7 +550,12 @@  uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size)
     return data;
 }
 
+#if FF_API_BUFFER_SIZE_T
 int av_packet_unpack_dictionary(const uint8_t *data, int size, AVDictionary **dict)
+#else
+int av_packet_unpack_dictionary(const uint8_t *data, size_t size,
+                                AVDictionary **dict)
+#endif
 {
     const uint8_t *end;
     int ret;
diff --git a/libavcodec/packet.h b/libavcodec/packet.h
index da4377e09f..ca18ae631f 100644
--- a/libavcodec/packet.h
+++ b/libavcodec/packet.h
@@ -647,7 +647,11 @@  const char *av_packet_side_data_name(enum AVPacketSideDataType type);
  * @param size pointer to store the size of the returned data
  * @return pointer to data if successful, NULL otherwise
  */
+#if FF_API_BUFFER_SIZE_T
 uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size);
+#else
+uint8_t *av_packet_pack_dictionary(AVDictionary *dict, size_t *size);
+#endif
 /**
  * Unpack a dictionary from side_data.
  *
@@ -656,8 +660,12 @@  uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size);
  * @param dict the metadata storage dictionary
  * @return 0 on success, < 0 on failure
  */
+#if FF_API_BUFFER_SIZE_T
 int av_packet_unpack_dictionary(const uint8_t *data, int size, AVDictionary **dict);
-
+#else
+int av_packet_unpack_dictionary(const uint8_t *data, size_t size,
+                                AVDictionary **dict);
+#endif
 
 /**
  * Convenience function to free all the side data stored.
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 40c3dae968..79d96cd620 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -922,7 +922,6 @@  HRESULT decklink_input_callback::VideoInputFrameArrived(
                     const char *tc = av_timecode_make_string(&tcr, tcstr, 0);
                     if (tc) {
                         AVDictionary* metadata_dict = NULL;
-                        int metadata_len;
                         uint8_t* packed_metadata;
 
                         if (av_cmp_q(ctx->video_st->r_frame_rate, av_make_q(60, 1)) < 1) {
@@ -937,6 +936,7 @@  HRESULT decklink_input_callback::VideoInputFrameArrived(
                         }
 
                         if (av_dict_set(&metadata_dict, "timecode", tc, 0) >= 0) {
+                            buffer_size_t metadata_len;
                             packed_metadata = av_packet_pack_dictionary(metadata_dict, &metadata_len);
                             av_dict_free(&metadata_dict);
                             if (packed_metadata) {
diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
index 94ad03268a..fdadff3f7f 100644
--- a/libavdevice/lavfi.c
+++ b/libavdevice/lavfi.c
@@ -446,7 +446,7 @@  static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
 
     frame_metadata = frame->metadata;
     if (frame_metadata) {
-        int size;
+        buffer_size_t size;
         uint8_t *metadata = av_packet_pack_dictionary(frame_metadata, &size);
 
         if (!metadata) {
diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index 6d5b9914f9..32d4a99010 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -626,7 +626,7 @@  static int concat_read_packet(AVFormatContext *avf, AVPacket *pkt)
            av_ts2str(pkt->pts), av_ts2timestr(pkt->pts, &st->time_base),
            av_ts2str(pkt->dts), av_ts2timestr(pkt->dts, &st->time_base));
     if (cat->cur_file->metadata) {
-        int metadata_len;
+        buffer_size_t metadata_len;
         char* packed_metadata = av_packet_pack_dictionary(cat->cur_file->metadata, &metadata_len);
         if (!packed_metadata)
             return AVERROR(ENOMEM);
diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
index 6c78dada8c..be7149bb2f 100644
--- a/libavformat/img2dec.c
+++ b/libavformat/img2dec.c
@@ -381,9 +381,10 @@  int ff_img_read_header(AVFormatContext *s1)
  * as a dictionary, so it can be used by filters like 'drawtext'.
  */
 static int add_filename_as_pkt_side_data(char *filename, AVPacket *pkt) {
-    int metadata_len, ret;
     AVDictionary *d = NULL;
     char *packed_metadata = NULL;
+    buffer_size_t metadata_len;
+    int ret;
 
     av_dict_set(&d, "lavf.image2dec.source_path", filename, 0);
     av_dict_set(&d, "lavf.image2dec.source_basename", av_basename(filename), 0);
diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h
index 629a1d6262..bf982bfe1c 100644
--- a/libavformat/oggdec.h
+++ b/libavformat/oggdec.h
@@ -87,7 +87,7 @@  struct ogg_stream {
     int start_trimming; ///< set the number of packets to drop from the start
     int end_trimming; ///< set the number of packets to drop from the end
     uint8_t *new_metadata;
-    unsigned int new_metadata_size;
+    buffer_size_t new_metadata_size;
     void *private;
 };