Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
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 14:58:51 +0200
Message-ID: <AS8P250MB0744D40206C0FFE40ED43BF18F12A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <a4ffed08-104a-4d73-1606-9a8acf6c710e@gmail.com>

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

  reply	other threads:[~2023-08-09 12:57 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
2023-08-09 10:08   ` Nicolas George
2023-08-09 11:32     ` James Almer
2023-08-09 12:58       ` Andreas Rheinhardt [this message]
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=AS8P250MB0744D40206C0FFE40ED43BF18F12A@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