Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 447cf2f

Browse files
committed
Remove manual tracking of file position in pg_dump/pg_backup_custom.c.
We do not really need to track the file position by hand. We were already relying on ftello() whenever the archive file is seekable, while if it's not seekable we don't need the file position info anyway because we're not going to be able to re-write the TOC. Moreover, that tracking was buggy since it failed to account for the effects of fseeko(). Somewhat remarkably, that seems not to have made for any live bugs up to now. We could fix the oversights, but it seems better to just get rid of the whole error-prone mess. In itself this is merely code cleanup. However, it's necessary infrastructure for an upcoming bug-fix patch (because that code *does* need valid file position after fseeko). The bug fix needs to go back as far as v12; hence, back-patch that far. Discussion: https://postgr.es/m/CALBH9DDuJ+scZc4MEvw5uO-=vRyR2=QF9+Yh=3hPEnKHWfS81A@mail.gmail.com
1 parent 49eb968 commit 447cf2f

File tree

1 file changed

+14
-39
lines changed

1 file changed

+14
-39
lines changed

src/bin/pg_dump/pg_backup_custom.c

+14-39
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,12 @@ typedef struct
7070
{
7171
CompressorState *cs;
7272
int hasSeek;
73-
pgoff_t filePos;
74-
pgoff_t dataStart;
7573
} lclContext;
7674

7775
typedef struct
7876
{
7977
int dataState;
80-
pgoff_t dataPos;
78+
pgoff_t dataPos; /* valid only if dataState=K_OFFSET_POS_SET */
8179
} lclTocEntry;
8280

8381

@@ -144,8 +142,6 @@ InitArchiveFmt_Custom(ArchiveHandle *AH)
144142
AH->lo_buf_size = LOBBUFSIZE;
145143
AH->lo_buf = (void *) pg_malloc(LOBBUFSIZE);
146144

147-
ctx->filePos = 0;
148-
149145
/*
150146
* Now open the file
151147
*/
@@ -185,7 +181,6 @@ InitArchiveFmt_Custom(ArchiveHandle *AH)
185181

186182
ReadHead(AH);
187183
ReadToc(AH);
188-
ctx->dataStart = _getFilePos(AH, ctx);
189184
}
190185

191186
}
@@ -291,7 +286,8 @@ _StartData(ArchiveHandle *AH, TocEntry *te)
291286
lclTocEntry *tctx = (lclTocEntry *) te->formatData;
292287

293288
tctx->dataPos = _getFilePos(AH, ctx);
294-
tctx->dataState = K_OFFSET_POS_SET;
289+
if (tctx->dataPos >= 0)
290+
tctx->dataState = K_OFFSET_POS_SET;
295291

296292
_WriteByte(AH, BLK_DATA); /* Block type */
297293
WriteInt(AH, te->dumpId); /* For sanity check */
@@ -352,7 +348,8 @@ _StartBlobs(ArchiveHandle *AH, TocEntry *te)
352348
lclTocEntry *tctx = (lclTocEntry *) te->formatData;
353349

354350
tctx->dataPos = _getFilePos(AH, ctx);
355-
tctx->dataState = K_OFFSET_POS_SET;
351+
if (tctx->dataPos >= 0)
352+
tctx->dataState = K_OFFSET_POS_SET;
356353

357354
_WriteByte(AH, BLK_BLOBS); /* Block type */
358355
WriteInt(AH, te->dumpId); /* For sanity check */
@@ -553,7 +550,6 @@ _skipBlobs(ArchiveHandle *AH)
553550
static void
554551
_skipData(ArchiveHandle *AH)
555552
{
556-
lclContext *ctx = (lclContext *) AH->formatData;
557553
size_t blkLen;
558554
char *buf = NULL;
559555
int buflen = 0;
@@ -577,8 +573,6 @@ _skipData(ArchiveHandle *AH)
577573
fatal("could not read from input file: %m");
578574
}
579575

580-
ctx->filePos += blkLen;
581-
582576
blkLen = ReadInt(AH);
583577
}
584578

@@ -596,12 +590,10 @@ _skipData(ArchiveHandle *AH)
596590
static int
597591
_WriteByte(ArchiveHandle *AH, const int i)
598592
{
599-
lclContext *ctx = (lclContext *) AH->formatData;
600593
int res;
601594

602595
if ((res = fputc(i, AH->FH)) == EOF)
603596
WRITE_ERROR_EXIT;
604-
ctx->filePos += 1;
605597

606598
return 1;
607599
}
@@ -617,13 +609,11 @@ _WriteByte(ArchiveHandle *AH, const int i)
617609
static int
618610
_ReadByte(ArchiveHandle *AH)
619611
{
620-
lclContext *ctx = (lclContext *) AH->formatData;
621612
int res;
622613

623614
res = getc(AH->FH);
624615
if (res == EOF)
625616
READ_ERROR_EXIT(AH->FH);
626-
ctx->filePos += 1;
627617
return res;
628618
}
629619

@@ -637,11 +627,8 @@ _ReadByte(ArchiveHandle *AH)
637627
static void
638628
_WriteBuf(ArchiveHandle *AH, const void *buf, size_t len)
639629
{
640-
lclContext *ctx = (lclContext *) AH->formatData;
641-
642630
if (fwrite(buf, 1, len, AH->FH) != len)
643631
WRITE_ERROR_EXIT;
644-
ctx->filePos += len;
645632
}
646633

647634
/*
@@ -654,11 +641,8 @@ _WriteBuf(ArchiveHandle *AH, const void *buf, size_t len)
654641
static void
655642
_ReadBuf(ArchiveHandle *AH, void *buf, size_t len)
656643
{
657-
lclContext *ctx = (lclContext *) AH->formatData;
658-
659644
if (fread(buf, 1, len, AH->FH) != len)
660645
READ_ERROR_EXIT(AH->FH);
661-
ctx->filePos += len;
662646
}
663647

664648
/*
@@ -690,7 +674,6 @@ _CloseArchive(ArchiveHandle *AH)
690674
if (tpos < 0 && ctx->hasSeek)
691675
fatal("could not determine seek position in archive file: %m");
692676
WriteToc(AH);
693-
ctx->dataStart = _getFilePos(AH, ctx);
694677
WriteDataChunks(AH, NULL);
695678

696679
/*
@@ -864,30 +847,24 @@ _WorkerJobRestoreCustom(ArchiveHandle *AH, TocEntry *te)
864847

865848
/*
866849
* Get the current position in the archive file.
850+
*
851+
* With a non-seekable archive file, we may not be able to obtain the
852+
* file position. If so, just return -1. It's not too important in
853+
* that case because we won't be able to rewrite the TOC to fill in
854+
* data block offsets anyway.
867855
*/
868856
static pgoff_t
869857
_getFilePos(ArchiveHandle *AH, lclContext *ctx)
870858
{
871859
pgoff_t pos;
872860

873-
if (ctx->hasSeek)
861+
pos = ftello(AH->FH);
862+
if (pos < 0)
874863
{
875-
/*
876-
* Prior to 1.7 (pg7.3) we relied on the internally maintained
877-
* pointer. Now we rely on ftello() always, unless the file has been
878-
* found to not support it. For debugging purposes, print a warning
879-
* if the internal pointer disagrees, so that we're more likely to
880-
* notice if something's broken about the internal position tracking.
881-
*/
882-
pos = ftello(AH->FH);
883-
if (pos < 0)
864+
/* Not expected if we found we can seek. */
865+
if (ctx->hasSeek)
884866
fatal("could not determine seek position in archive file: %m");
885-
886-
if (pos != ctx->filePos)
887-
pg_log_warning("ftell mismatch with expected position -- ftell used");
888867
}
889-
else
890-
pos = ctx->filePos;
891868
return pos;
892869
}
893870

@@ -899,7 +876,6 @@ _getFilePos(ArchiveHandle *AH, lclContext *ctx)
899876
static void
900877
_readBlockHeader(ArchiveHandle *AH, int *type, int *id)
901878
{
902-
lclContext *ctx = (lclContext *) AH->formatData;
903879
int byt;
904880

905881
/*
@@ -920,7 +896,6 @@ _readBlockHeader(ArchiveHandle *AH, int *type, int *id)
920896
*id = 0; /* don't return an uninitialized value */
921897
return;
922898
}
923-
ctx->filePos += 1;
924899
}
925900

926901
*id = ReadInt(AH);

0 commit comments

Comments
 (0)