Hi, While examining pg_dump code I found some room for improvement:
1. I found a write-only field ArchiveHandle.lookaheadSize which can safely be removed. 2. I found magic constants 512 which should be replaced with TAR_BLOCK_SIZE. Patches are attached. -- Best regards, Aleksander Alekseev
From 05391a967a025ceba396b67a86b7f651c0627782 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev <[email protected]> Date: Tue, 18 Aug 2026 17:44:08 +0300 Subject: [PATCH v1 1/2] pg_dump: remove unused ArchiveHandle.lookaheadSize field _discoverArchiveFormat() set this field to the size it passed to pg_malloc0(), but no code has ever read it since it was added. The other places that need the capacity of the lookahead buffer spell out 512 directly, and rightly so: the value is really the tar block size, which is what makes the buffer large enough to hold a tar header for isValidTarHeader() to inspect. Oversight in e8f69be054e. Author: Aleksander Alekseev <[email protected]> Reviewed-by: TODO FIXME Discussion: TODO FIXME --- src/bin/pg_dump/pg_backup_archiver.c | 1 - src/bin/pg_dump/pg_backup_archiver.h | 1 - 2 files changed, 2 deletions(-) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index d7da3fc4325..c2aa7e5669e 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -2252,7 +2252,6 @@ _discoverArchiveFormat(ArchiveHandle *AH) pg_free(AH->lookahead); AH->readHeader = 0; - AH->lookaheadSize = 512; AH->lookahead = pg_malloc0(512); AH->lookaheadLen = 0; AH->lookaheadPos = 0; diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h index c1528d78853..e2b1cff610e 100644 --- a/src/bin/pg_dump/pg_backup_archiver.h +++ b/src/bin/pg_dump/pg_backup_archiver.h @@ -244,7 +244,6 @@ struct _archiveHandle int readHeader; /* Set if we already read "PGDMP" marker */ char *lookahead; /* Buffer used when reading header to discover * format */ - size_t lookaheadSize; /* Allocated size of buffer */ size_t lookaheadLen; /* Length of valid data in lookahead */ size_t lookaheadPos; /* Current read position in lookahead buffer */ -- 2.43.0
From 28d3ae6bc469f237f6ad4d31f06e8caf81111089 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev <[email protected]> Date: Tue, 18 Aug 2026 17:55:45 +0300 Subject: [PATCH v1 2/2] pg_dump: use TAR_BLOCK_SIZE instead of a hardcoded 512 _discoverArchiveFormat() reads the start of the input file into a lookahead buffer and, unless the file turns out to be a custom-format archive or a text dump, hands that buffer to isValidTarHeader(). The latter examines exactly TAR_BLOCK_SIZE bytes, since tarChecksum() sums the whole tar block, so the buffer size, the amount read and the completeness check all have to be one tar block. Spell that out rather than repeating the literal 512 three times. The rest of the tar code already does it this way, and so does the other caller of isValidTarHeader() in astreamer_tar.c, which even asserts that its buffer is TAR_BLOCK_SIZE long. pgtar.h was already included here. This is only cosmetic, no behavior changes. Author: Aleksander Alekseev <[email protected]> Reviewed-by: TODO FIXME Discussion: TODO FIXME --- src/bin/pg_dump/pg_backup_archiver.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index c2aa7e5669e..88f6990c38f 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -2252,7 +2252,7 @@ _discoverArchiveFormat(ArchiveHandle *AH) pg_free(AH->lookahead); AH->readHeader = 0; - AH->lookahead = pg_malloc0(512); + AH->lookahead = pg_malloc0(TAR_BLOCK_SIZE); AH->lookaheadLen = 0; AH->lookaheadPos = 0; @@ -2323,9 +2323,10 @@ _discoverArchiveFormat(ArchiveHandle *AH) { /* * *Maybe* we have a tar archive format file or a text dump ... So, - * read first 512 byte header... + * read first tar block, which is what isValidTarHeader() inspects. */ - cnt = fread(&AH->lookahead[AH->lookaheadLen], 1, 512 - AH->lookaheadLen, fh); + cnt = fread(&AH->lookahead[AH->lookaheadLen], 1, + TAR_BLOCK_SIZE - AH->lookaheadLen, fh); /* read failure is checked below */ AH->lookaheadLen += cnt; @@ -2340,7 +2341,7 @@ _discoverArchiveFormat(ArchiveHandle *AH) pg_fatal("input file appears to be a text format dump. Please use psql."); } - if (AH->lookaheadLen != 512) + if (AH->lookaheadLen != TAR_BLOCK_SIZE) { if (feof(fh)) pg_fatal("input file does not appear to be a valid archive (too short?)"); -- 2.43.0
