benign bug in BufFileLoadBuffer / incorrect sizeof

Started by Tomas Vondra12 months ago2 messages

tomas@vondra.me

12 months ago

1 attachment(s)

Hi,

While experimenting with some changes in BufFile, I noticed a harmless
bug in BufFileLoadBuffer. It calls sizeof on the whole PGAlignedBuffer,
instead of just on the "data" field. It's benign because the "data" is
the largest part of the union, so the sizes are equal.

But it's still confusing, it took me a while my experimental patch
fails. So I think it'd be good to correct it.

regards

--
Tomas Vondra

Attachments:

buffile-sizeof-fix.patchtext/x-patch; charset=UTF-8; name=buffile-sizeof-fix.patchDownload

diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c
index 6449f82a72b..a4541b87b8f 100644
--- a/src/backend/storage/file/buffile.c
+++ b/src/backend/storage/file/buffile.c
@@ -459,7 +459,7 @@ BufFileLoadBuffer(BufFile *file)
 	 */
 	file->nbytes = FileRead(thisfile,
 							file->buffer.data,
-							sizeof(file->buffer),
+							sizeof(file->buffer.data),
 							file->curOffset,
 							WAIT_EVENT_BUFFILE_READ);
 	if (file->nbytes < 0)

Daniel Gustafsson

daniel@yesql.se

12 months ago

In reply to: Tomas Vondra (#1)

Re: benign bug in BufFileLoadBuffer / incorrect sizeof

On 12 Jan 2025, at 17:10, Tomas Vondra <tomas@vondra.me> wrote:

Hi,

While experimenting with some changes in BufFile, I noticed a harmless
bug in BufFileLoadBuffer. It calls sizeof on the whole PGAlignedBuffer,
instead of just on the "data" field. It's benign because the "data" is
the largest part of the union, so the sizes are equal.

But it's still confusing, it took me a while my experimental patch
fails. So I think it'd be good to correct it.

Nice catch, patch LGTM.

--
Daniel Gustafsson