[FFmpeg-devel] [PATCH 1/7] avutil/bprint: Don't use value of AV_BPRINT_SIZE_AUTOMATIC directly

[FFmpeg-devel] [PATCH 1/7] avutil/bprint: Don't use value of AV_BPRINT_SIZE_AUTOMATIC directly

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavutil/bprint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavutil/bprint.c b/libavutil/bprint.c
index 5b540ebc9e..23998a8b02 100644
--- a/libavutil/bprint.c
+++ b/libavutil/bprint.c
@@ -71,7 +71,7 @@ void av_bprint_init(AVBPrint *buf, unsigned size_init, unsigned size_max)
     unsigned size_auto = (char *)buf + sizeof(*buf) -
                          buf->reserved_internal_buffer;
 
-    if (size_max == 1)
+    if (size_max == AV_BPRINT_SIZE_AUTOMATIC)
         size_max = size_auto;
     buf->str      = buf->reserved_internal_buffer;
     buf->len      = 0;
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/7] avutil/bprint: Allow size == 0 in av_bprint_init_for_buffer()

[Andreas Rheinhardt]

The AVBPrint API guarantees that the string buffer is always
zero-terminated; in order to honour this guarantee, there
obviously must be a string buffer at all and it must have
a size >= 1. Therefore av_bprint_init_for_buffer() treats
passing a NULL buffer or size == 0 as invalid data that
leads to undefined behaviour, namely NPD in case NULL is provided
or a write to a buffer of size 0 in case size == 0.

But it would be easy to support this, namely by using the internal
buffer with AV_BPRINT_SIZE_COUNT_ONLY in case size == 0.

There is a reason to allow this: Several functions like
av_channel_(description|name) are actually wrappers
around corresponding AVBPrint functions. They accept user
provided buffers and are supposed to return the required
size of the buffer, which would allow the user to call
it once to get the required buffer size and call it once
more after having allocated the buffer.
If av_bprint_init_for_buffer() treats size == 0 as invalid,
all these users would need to check for this themselves
and basically add the same codeblock that this patch
adds to av_bprint_init_for_buffer().

This change is in line with e.g. snprintf() which also allows
the pointer to be NULL in case size is zero.

This fixes Coverity issues #1503074, #1503076 and #1503082;
all of these issues are about providing NULL to the channel-layout
functions that are wrappers around AVBPrint versions.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Missing lavu minor version bump.

 libavutil/bprint.c | 5 +++++
 libavutil/bprint.h | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/libavutil/bprint.c b/libavutil/bprint.c
index 23998a8b02..4e9571715c 100644
--- a/libavutil/bprint.c
+++ b/libavutil/bprint.c
@@ -84,6 +84,11 @@ void av_bprint_init(AVBPrint *buf, unsigned size_init, unsigned size_max)
 
 void av_bprint_init_for_buffer(AVBPrint *buf, char *buffer, unsigned size)
 {
+    if (size == 0) {
+        av_bprint_init(buf, 0, AV_BPRINT_SIZE_COUNT_ONLY);
+        return;
+    }
+
     buf->str      = buffer;
     buf->len      = 0;
     buf->size     = size;
diff --git a/libavutil/bprint.h b/libavutil/bprint.h
index f27d30f723..8559745478 100644
--- a/libavutil/bprint.h
+++ b/libavutil/bprint.h
@@ -144,6 +144,9 @@ void av_bprint_init(AVBPrint *buf, unsigned size_init, unsigned size_max);
  * Init a print buffer using a pre-existing buffer.
  *
  * The buffer will not be reallocated.
+ * In case size equals zero, the AVBPrint will be initialized to use
+ * the internal buffer as if using AV_BPRINT_SIZE_COUNT_ONLY with
+ * av_bprint_init().
  *
  * @param buf     buffer structure to init
  * @param buffer  byte buffer to use for the string data
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/7] avutil/channel_layout: Account for \0 in sizes

[Andreas Rheinhardt]

av_channel_name(), av_channel_description() and
av_channel_layout_describe() are supposed to return the size
of the needed buffer to allow the user to check for truncation;
the documentation ("If the returned value is bigger than buf_size,
then the string was truncated.") confirms that size does not
mean strlen.

Yet the AVBPrint API, i.e. AVBPrint.len, does not account for
the terminating '\0'. Therefore the returned length is off by one.

Furthermore, also check for whether the returned value actually
fits in an int (which is the return value of these functions).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Missing lavu micro version bump.

 libavutil/channel_layout.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index e2f7512254..9b581ae6b3 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -108,7 +108,9 @@ int av_channel_name(char *buf, size_t buf_size, enum AVChannel channel_id)
     av_bprint_init_for_buffer(&bp, buf, buf_size);
     av_channel_name_bprint(&bp, channel_id);
 
-    return bp.len;
+    if (bp.len >= INT_MAX)
+        return AVERROR(ERANGE);
+    return bp.len + 1;
 }
 
 void av_channel_description_bprint(AVBPrint *bp, enum AVChannel channel_id)
@@ -135,7 +137,9 @@ int av_channel_description(char *buf, size_t buf_size, enum AVChannel channel_id
     av_bprint_init_for_buffer(&bp, buf, buf_size);
     av_channel_description_bprint(&bp, channel_id);
 
-    return bp.len;
+    if (bp.len >= INT_MAX)
+        return AVERROR(ERANGE);
+    return bp.len + 1;
 }
 
 enum AVChannel av_channel_from_string(const char *str)
@@ -789,7 +793,9 @@ int av_channel_layout_describe(const AVChannelLayout *channel_layout,
     if (ret < 0)
         return ret;
 
-    return bp.len;
+    if (bp.len >= INT_MAX)
+        return AVERROR(ERANGE);
+    return bp.len + 1;
 }
 
 enum AVChannel
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 4/7] avutil/tests/channel_layout: Also test non-AVBPrint variants

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavutil/tests/channel_layout.c | 108 +++++++++++++++++++++++++------
 1 file changed, 90 insertions(+), 18 deletions(-)

diff --git a/libavutil/tests/channel_layout.c b/libavutil/tests/channel_layout.c
index 5516db0904..665ae6e277 100644
--- a/libavutil/tests/channel_layout.c
+++ b/libavutil/tests/channel_layout.c
@@ -19,32 +19,104 @@
  */
 
 #include "libavutil/channel_layout.c"
+#include "libavutil/mem.h"
+
+#define BPRINT_ARGS1(bp, ...)     (bp), __VA_ARGS__
+#define BPRINT_ARGS0(bp, ...)     __VA_ARGS__, (bp)
+#define ORD_ARGS1(str, size, ...) (str), (size), __VA_ARGS__
+#define ORD_ARGS0(str, size, ...) __VA_ARGS__, (str), (size)
+
+// This macro presumes the AVBPrint to have been cleared before usage.
+#define CMP_BPRINT_AND_NONBPRINT(bp, func_name, ARG_ORDER, ...) do {       \
+    char *str;                                                             \
+    int size;                                                              \
+    func_name ## _bprint(BPRINT_ARGS ## ARG_ORDER((bp), __VA_ARGS__));     \
+    if (strlen((bp)->str) != (bp)->len) {                                  \
+        printf("strlen of AVBPrint-string returned by "#func_name"_bprint" \
+               " differs from AVBPrint.len: %"SIZE_SPECIFIER" vs. %u\n",   \
+               strlen((bp)->str), (bp)->len);                              \
+        break;                                                             \
+    }                                                                      \
+    size = func_name(ORD_ARGS ## ARG_ORDER(NULL, 0, __VA_ARGS__));         \
+    if (size <= 0) {                                                       \
+        printf(#func_name " returned %d\n", size);                         \
+        break;                                                             \
+    }                                                                      \
+    if ((bp)->len != size - 1) {                                           \
+        printf("Return value %d of " #func_name " inconsistent with length"\
+               " %u obtained from corresponding bprint version\n",         \
+               size, (bp)->len);                                           \
+        break;                                                             \
+    }                                                                      \
+    str = av_malloc(size);                                                 \
+    if (!str) {                                                            \
+        printf("string of size %d could not be allocated.\n", size);       \
+        break;                                                             \
+    }                                                                      \
+    size = func_name(ORD_ARGS ## ARG_ORDER(str, size, __VA_ARGS__));       \
+    if (size <= 0 || (bp)->len != size - 1) {                              \
+        printf("Return value %d of " #func_name " inconsistent with length"\
+               " %d obtained in first pass.\n", size, (bp)->len);          \
+        av_free(str);                                                      \
+        break;                                                             \
+    }                                                                      \
+    if (strcmp(str, (bp)->str)) {                                          \
+        printf("Ordinary and _bprint versions of "#func_name" disagree: "  \
+               "'%s' vs. '%s'\n", str, (bp)->str);                         \
+        av_free(str);                                                      \
+        break;                                                             \
+    }                                                                      \
+    av_free(str);                                                          \
+    } while (0)
+
+
+static void channel_name(AVBPrint *bp, enum AVChannel channel)
+{
+    av_bprint_clear(bp);
+    CMP_BPRINT_AND_NONBPRINT(bp, av_channel_name, 1, channel);
+}
+
+static void channel_description(AVBPrint *bp, enum AVChannel channel)
+{
+    av_bprint_clear(bp);
+    CMP_BPRINT_AND_NONBPRINT(bp, av_channel_description, 1, channel);
+}
+
+static void channel_layout_from_mask(AVChannelLayout *layout,
+                                     AVBPrint *bp, uint64_t channel_layout)
+{
+    av_channel_layout_uninit(layout);
+    av_bprint_clear(bp);
+    if (!av_channel_layout_from_mask(layout, channel_layout) &&
+         av_channel_layout_check(layout))
+        CMP_BPRINT_AND_NONBPRINT(bp, av_channel_layout_describe, 0, layout);
+    else
+        av_bprintf(bp, "fail");
+}
+
+static void channel_layout_from_string(AVChannelLayout *layout,
+                                       AVBPrint *bp, const char *channel_layout)
+{
+    av_channel_layout_uninit(layout);
+    av_bprint_clear(bp);
+    if (!av_channel_layout_from_string(layout, channel_layout) &&
+         av_channel_layout_check(layout))
+        CMP_BPRINT_AND_NONBPRINT(bp, av_channel_layout_describe, 0, layout);
+    else
+        av_bprintf(bp, "fail");
+}
 
 #define CHANNEL_NAME(x)                                                    \
-    av_bprint_clear(&bp);                                                  \
-    av_channel_name_bprint(&bp, x);
+    channel_name(&bp, (x));
 
 #define CHANNEL_DESCRIPTION(x)                                             \
-    av_bprint_clear(&bp);                                                  \
-    av_channel_description_bprint(&bp, x);
+    channel_description(&bp, (x));
 
 #define CHANNEL_LAYOUT_FROM_MASK(x)                                        \
-    av_channel_layout_uninit(&layout);                                     \
-    av_bprint_clear(&bp);                                                  \
-    if (!av_channel_layout_from_mask(&layout, x) &&                        \
-         av_channel_layout_check(&layout))                                 \
-        av_channel_layout_describe_bprint(&layout, &bp);                   \
-    else                                                                   \
-        av_bprintf(&bp, "fail");
+    channel_layout_from_mask(&layout, &bp, (x));
 
 #define CHANNEL_LAYOUT_FROM_STRING(x)                                      \
-    av_channel_layout_uninit(&layout);                                     \
-    av_bprint_clear(&bp);                                                  \
-    if (!av_channel_layout_from_string(&layout, x) &&                      \
-         av_channel_layout_check(&layout))                                 \
-        av_channel_layout_describe_bprint(&layout, &bp);                   \
-    else                                                                   \
-        av_bprintf(&bp, "fail");
+    channel_layout_from_string(&layout, &bp, (x));
 
 #define CHANNEL_LAYOUT_CHANNEL_FROM_INDEX(x)                               \
     ret = av_channel_layout_channel_from_index(&layout, x);                \
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 5/7] avutil/tests/channel_layout: Don't include lavu/channel_layout.c

[Andreas Rheinhardt]

This test does not need access to the internals of said compilation
unit.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavutil/tests/channel_layout.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavutil/tests/channel_layout.c b/libavutil/tests/channel_layout.c
index 665ae6e277..19f552f8bc 100644
--- a/libavutil/tests/channel_layout.c
+++ b/libavutil/tests/channel_layout.c
@@ -18,7 +18,13 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-#include "libavutil/channel_layout.c"
+#include <inttypes.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "libavutil/bprint.h"
+#include "libavutil/channel_layout.h"
+#include "libavutil/internal.h"
 #include "libavutil/mem.h"
 
 #define BPRINT_ARGS1(bp, ...)     (bp), __VA_ARGS__
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 6/7] avutil/tests/channel_layout: Test av_channel_layout_copy()

[Andreas Rheinhardt]

Specifically, test copying a channel layout with custom order,
so that the allocation codepath of av_channel_layout_copy()
is executed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavutil/tests/channel_layout.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/libavutil/tests/channel_layout.c b/libavutil/tests/channel_layout.c
index 19f552f8bc..c537e7e710 100644
--- a/libavutil/tests/channel_layout.c
+++ b/libavutil/tests/channel_layout.c
@@ -150,7 +150,7 @@ static void channel_layout_from_string(AVChannelLayout *layout,
 int main(void)
 {
     const AVChannelLayout *playout;
-    AVChannelLayout layout = { 0 };
+    AVChannelLayout layout = { 0 }, layout2 = { 0 };
     AVBPrint bp;
     void *iter = NULL;
     uint64_t mask;
@@ -324,6 +324,15 @@ int main(void)
     CHANNEL_LAYOUT_FROM_STRING("FR+FL@Foo+USR63@Foo");
     printf("With \"FR+FL@Foo+USR63@Foo\": %33s\n", bp.str);
 
+    ret = av_channel_layout_copy(&layout2, &layout);
+    if (ret < 0) {
+        printf("Copying channel layout \"FR+FL@Foo+USR63@Foo\" failed; "
+               "ret %d\n", ret);
+    }
+    ret = av_channel_layout_compare(&layout, &layout2);
+    if (ret)
+        printf("Channel layout and its copy compare unequal; ret: %d\n", ret);
+
     printf("\nTesting av_channel_layout_index_from_string\n");
     CHANNEL_LAYOUT_INDEX_FROM_STRING("FR");
     printf("On \"FR+FL@Foo+USR63@Foo\" layout with \"FR\": %18d\n", ret);
@@ -425,6 +434,7 @@ int main(void)
     printf("On \"ambisonic 2+stereo\" layout with AV_CH_LAYOUT_QUAD:    0x%"PRIx64"\n", mask);
 
     av_channel_layout_uninit(&layout);
+    av_channel_layout_uninit(&layout2);
     av_bprint_finalize(&bp, NULL);
 
     return 0;
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 7/7] avcodec/amfenc: Fix declaration-after-statement warning

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/amfenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
index cb48f8c273..518b8396e7 100644
--- a/libavcodec/amfenc.c
+++ b/libavcodec/amfenc.c
@@ -720,10 +720,10 @@ int ff_amf_receive_packet(AVCodecContext *avctx, AVPacket *avpkt)
         if (!avpkt->data && !avpkt->buf) {
             res_query = ctx->encoder->pVtbl->QueryOutput(ctx->encoder, &data);
             if (data) {
-                query_output_data_flag = 1;
                 // copy data to packet
                 AMFBuffer *buffer;
                 AMFGuid guid = IID_AMFBuffer();
+                query_output_data_flag = 1;
                 data->pVtbl->QueryInterface(data, &guid, (void**)&buffer); // query for buffer interface
                 ret = amf_copy_buffer(avctx, avpkt, buffer);
 
-- 
2.34.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/7] avutil/bprint: Allow size == 0 in av_bprint_init_for_buffer()

[Andreas Rheinhardt]

Andreas Rheinhardt:
> The AVBPrint API guarantees that the string buffer is always
> zero-terminated; in order to honour this guarantee, there
> obviously must be a string buffer at all and it must have
> a size >= 1. Therefore av_bprint_init_for_buffer() treats
> passing a NULL buffer or size == 0 as invalid data that
> leads to undefined behaviour, namely NPD in case NULL is provided
> or a write to a buffer of size 0 in case size == 0.
> 
> But it would be easy to support this, namely by using the internal
> buffer with AV_BPRINT_SIZE_COUNT_ONLY in case size == 0.
> 
> There is a reason to allow this: Several functions like
> av_channel_(description|name) are actually wrappers
> around corresponding AVBPrint functions. They accept user
> provided buffers and are supposed to return the required
> size of the buffer, which would allow the user to call
> it once to get the required buffer size and call it once
> more after having allocated the buffer.
> If av_bprint_init_for_buffer() treats size == 0 as invalid,
> all these users would need to check for this themselves
> and basically add the same codeblock that this patch
> adds to av_bprint_init_for_buffer().
> 
> This change is in line with e.g. snprintf() which also allows
> the pointer to be NULL in case size is zero.
> 
> This fixes Coverity issues #1503074, #1503076 and #1503082;
> all of these issues are about providing NULL to the channel-layout
> functions that are wrappers around AVBPrint versions.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> Missing lavu minor version bump.
> 
>  libavutil/bprint.c | 5 +++++
>  libavutil/bprint.h | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/libavutil/bprint.c b/libavutil/bprint.c
> index 23998a8b02..4e9571715c 100644
> --- a/libavutil/bprint.c
> +++ b/libavutil/bprint.c
> @@ -84,6 +84,11 @@ void av_bprint_init(AVBPrint *buf, unsigned size_init, unsigned size_max)
>  
>  void av_bprint_init_for_buffer(AVBPrint *buf, char *buffer, unsigned size)
>  {
> +    if (size == 0) {
> +        av_bprint_init(buf, 0, AV_BPRINT_SIZE_COUNT_ONLY);
> +        return;
> +    }
> +
>      buf->str      = buffer;
>      buf->len      = 0;
>      buf->size     = size;
> diff --git a/libavutil/bprint.h b/libavutil/bprint.h
> index f27d30f723..8559745478 100644
> --- a/libavutil/bprint.h
> +++ b/libavutil/bprint.h
> @@ -144,6 +144,9 @@ void av_bprint_init(AVBPrint *buf, unsigned size_init, unsigned size_max);
>   * Init a print buffer using a pre-existing buffer.
>   *
>   * The buffer will not be reallocated.
> + * In case size equals zero, the AVBPrint will be initialized to use
> + * the internal buffer as if using AV_BPRINT_SIZE_COUNT_ONLY with
> + * av_bprint_init().
>   *
>   * @param buf     buffer structure to init
>   * @param buffer  byte buffer to use for the string data

Ping for this patch (and the rest of this patchset). Will apply patches
2-5 in two days unless there are objections; will apply the rest today.

- Andreas

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/7] avutil/bprint: Allow size == 0 in av_bprint_init_for_buffer()

[Nicolas George]

Andreas Rheinhardt (12023-08-06):
> The AVBPrint API guarantees that the string buffer is always
> zero-terminated; in order to honour this guarantee, there
> obviously must be a string buffer at all and it must have
> a size >= 1. Therefore av_bprint_init_for_buffer() treats
> passing a NULL buffer or size == 0 as invalid data that
> leads to undefined behaviour, namely NPD in case NULL is provided
> or a write to a buffer of size 0 in case size == 0.
> 
> But it would be easy to support this, namely by using the internal
> buffer with AV_BPRINT_SIZE_COUNT_ONLY in case size == 0.
> 
> There is a reason to allow this: Several functions like
> av_channel_(description|name) are actually wrappers
> around corresponding AVBPrint functions. They accept user
> provided buffers and are supposed to return the required
> size of the buffer, which would allow the user to call
> it once to get the required buffer size and call it once
> more after having allocated the buffer.
> If av_bprint_init_for_buffer() treats size == 0 as invalid,
> all these users would need to check for this themselves
> and basically add the same codeblock that this patch
> adds to av_bprint_init_for_buffer().
> 
> This change is in line with e.g. snprintf() which also allows
> the pointer to be NULL in case size is zero.
> 
> This fixes Coverity issues #1503074, #1503076 and #1503082;
> all of these issues are about providing NULL to the channel-layout
> functions that are wrappers around AVBPrint versions.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> Missing lavu minor version bump.

Looks good to me.

The other patches in the series too, but I do not maintain the channel
layouts.

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".

Re: [FFmpeg-devel] [PATCH 2/7] avutil/bprint: Allow size == 0 in av_bprint_init_for_buffer()

[James Almer]

On 8/9/2023 7:08 AM, Nicolas George wrote:
> Andreas Rheinhardt (12023-08-06):
>> The AVBPrint API guarantees that the string buffer is always
>> zero-terminated; in order to honour this guarantee, there
>> obviously must be a string buffer at all and it must have
>> a size >= 1. Therefore av_bprint_init_for_buffer() treats
>> passing a NULL buffer or size == 0 as invalid data that
>> leads to undefined behaviour, namely NPD in case NULL is provided
>> or a write to a buffer of size 0 in case size == 0.
>>
>> But it would be easy to support this, namely by using the internal
>> buffer with AV_BPRINT_SIZE_COUNT_ONLY in case size == 0.
>>
>> There is a reason to allow this: Several functions like
>> av_channel_(description|name) are actually wrappers
>> around corresponding AVBPrint functions. They accept user
>> provided buffers and are supposed to return the required
>> size of the buffer, which would allow the user to call
>> it once to get the required buffer size and call it once
>> more after having allocated the buffer.
>> If av_bprint_init_for_buffer() treats size == 0 as invalid,
>> all these users would need to check for this themselves
>> and basically add the same codeblock that this patch
>> adds to av_bprint_init_for_buffer().
>>
>> This change is in line with e.g. snprintf() which also allows
>> the pointer to be NULL in case size is zero.
>>
>> This fixes Coverity issues #1503074, #1503076 and #1503082;
>> all of these issues are about providing NULL to the channel-layout
>> functions that are wrappers around AVBPrint versions.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> Missing lavu minor version bump.
> 
> Looks good to me.
> 
> The other patches in the series too, but I do not maintain the channel
> layouts.
> 
> Regards,

The layout patches are ok too.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/7] avutil/bprint: Allow size == 0 in av_bprint_init_for_buffer()

[Andreas Rheinhardt]

James Almer:
> On 8/9/2023 7:08 AM, Nicolas George wrote:
>> Andreas Rheinhardt (12023-08-06):
>>> The AVBPrint API guarantees that the string buffer is always
>>> zero-terminated; in order to honour this guarantee, there
>>> obviously must be a string buffer at all and it must have
>>> a size >= 1. Therefore av_bprint_init_for_buffer() treats
>>> passing a NULL buffer or size == 0 as invalid data that
>>> leads to undefined behaviour, namely NPD in case NULL is provided
>>> or a write to a buffer of size 0 in case size == 0.
>>>
>>> But it would be easy to support this, namely by using the internal
>>> buffer with AV_BPRINT_SIZE_COUNT_ONLY in case size == 0.
>>>
>>> There is a reason to allow this: Several functions like
>>> av_channel_(description|name) are actually wrappers
>>> around corresponding AVBPrint functions. They accept user
>>> provided buffers and are supposed to return the required
>>> size of the buffer, which would allow the user to call
>>> it once to get the required buffer size and call it once
>>> more after having allocated the buffer.
>>> If av_bprint_init_for_buffer() treats size == 0 as invalid,
>>> all these users would need to check for this themselves
>>> and basically add the same codeblock that this patch
>>> adds to av_bprint_init_for_buffer().
>>>
>>> This change is in line with e.g. snprintf() which also allows
>>> the pointer to be NULL in case size is zero.
>>>
>>> This fixes Coverity issues #1503074, #1503076 and #1503082;
>>> all of these issues are about providing NULL to the channel-layout
>>> functions that are wrappers around AVBPrint versions.
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>>> ---
>>> Missing lavu minor version bump.
>>
>> Looks good to me.
>>
>> The other patches in the series too, but I do not maintain the channel
>> layouts.
>>
>> Regards,
> 
> The layout patches are ok too.

Ok, then I'll already apply them tomorrow. Thanks for the reviews.

- Andreas

_______________________________________________
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".