From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 2/7] avutil/bprint: Allow size == 0 in av_bprint_init_for_buffer()
Date: Wed, 9 Aug 2023 11:00:40 +0200
Message-ID: <AS8P250MB0744E69D53483060F41B4CA18F12A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <AS8P250MB07441B7917A010E04C642A5D8F0FA@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM>
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".
next prev parent reply other threads:[~2023-08-09 8:59 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-06 10:10 [FFmpeg-devel] [PATCH 1/7] avutil/bprint: Don't use value of AV_BPRINT_SIZE_AUTOMATIC directly Andreas Rheinhardt
2023-08-06 10:13 ` [FFmpeg-devel] [PATCH 2/7] avutil/bprint: Allow size == 0 in av_bprint_init_for_buffer() Andreas Rheinhardt
2023-08-09 9:00 ` Andreas Rheinhardt [this message]
2023-08-09 10:08 ` Nicolas George
2023-08-09 11:32 ` James Almer
2023-08-09 12:58 ` Andreas Rheinhardt
2023-08-06 10:13 ` [FFmpeg-devel] [PATCH 3/7] avutil/channel_layout: Account for \0 in sizes Andreas Rheinhardt
2023-08-06 10:13 ` [FFmpeg-devel] [PATCH 4/7] avutil/tests/channel_layout: Also test non-AVBPrint variants Andreas Rheinhardt
2023-08-06 10:13 ` [FFmpeg-devel] [PATCH 5/7] avutil/tests/channel_layout: Don't include lavu/channel_layout.c Andreas Rheinhardt
2023-08-06 10:13 ` [FFmpeg-devel] [PATCH 6/7] avutil/tests/channel_layout: Test av_channel_layout_copy() Andreas Rheinhardt
2023-08-06 10:13 ` [FFmpeg-devel] [PATCH 7/7] avcodec/amfenc: Fix declaration-after-statement warning Andreas Rheinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AS8P250MB0744E69D53483060F41B4CA18F12A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM \
--to=andreas.rheinhardt@outlook.com \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git