On 01.12.22 09:25, Peter Eisentraut wrote:
Here are a couple of patches that clean up the internal File API and related things a bit:
Here are two follow-up patches that clean up some stuff related to the earlier patch set. I suspect these are all historically related.
0001-Remove-unnecessary-casts.patch Some code carefully cast all data buffer arguments for data write and read function calls to void *, even though the respective arguments are already void *. Remove this unnecessary clutter. 0002-Add-const-to-BufFileWrite.patch Make data buffer argument to BufFileWrite a const pointer and bubble this up to various callers and related APIs. This makes the APIs clearer and more consistent.
From cec7ec188f2473d983b048b9e1eddc7dbc3a4241 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Fri, 23 Dec 2022 08:35:42 +0100 Subject: [PATCH 1/2] Remove unnecessary casts Some code carefully cast all data buffer arguments for data write and read function calls to void *, even though the respective arguments are already void *. Remove this unnecessary clutter. --- src/backend/executor/nodeAgg.c | 6 +++--- src/backend/storage/file/buffile.c | 4 ++-- src/backend/utils/sort/logtape.c | 18 +++++++++--------- src/backend/utils/sort/tuplesort.c | 2 +- src/backend/utils/sort/tuplesortvariants.c | 16 ++++++++-------- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 30c9143183..f15bb83a1a 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2961,10 +2961,10 @@ hashagg_spill_tuple(AggState *aggstate, HashAggSpill *spill, tape = spill->partitions[partition]; - LogicalTapeWrite(tape, (void *) &hash, sizeof(uint32)); + LogicalTapeWrite(tape, &hash, sizeof(uint32)); total_written += sizeof(uint32); - LogicalTapeWrite(tape, (void *) tuple, tuple->t_len); + LogicalTapeWrite(tape, tuple, tuple->t_len); total_written += tuple->t_len; if (shouldFree) @@ -3029,7 +3029,7 @@ hashagg_batch_read(HashAggBatch *batch, uint32 *hashp) tuple->t_len = t_len; nread = LogicalTapeRead(tape, - (void *) ((char *) tuple + sizeof(uint32)), + (char *) tuple + sizeof(uint32), t_len - sizeof(uint32)); if (nread != t_len - sizeof(uint32)) ereport(ERROR, diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c index b0b4eeb3bd..b07cca28d6 100644 --- a/src/backend/storage/file/buffile.c +++ b/src/backend/storage/file/buffile.c @@ -607,7 +607,7 @@ BufFileRead(BufFile *file, void *ptr, size_t size) memcpy(ptr, file->buffer.data + file->pos, nthistime); file->pos += nthistime; - ptr = (void *) ((char *) ptr + nthistime); + ptr = (char *) ptr + nthistime; size -= nthistime; nread += nthistime; } @@ -655,7 +655,7 @@ BufFileWrite(BufFile *file, void *ptr, size_t size) file->pos += nthistime; if (file->nbytes < file->pos) file->nbytes = file->pos; - ptr = (void *) ((char *) ptr + nthistime); + ptr = (char *) ptr + nthistime; size -= nthistime; } } diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c index c384f98e13..9db220b7ea 100644 --- a/src/backend/utils/sort/logtape.c +++ b/src/backend/utils/sort/logtape.c @@ -319,7 +319,7 @@ ltsReadFillBuffer(LogicalTape *lt) datablocknum += lt->offsetBlockNumber; /* Read the block */ - ltsReadBlock(lt->tapeSet, datablocknum, (void *) thisbuf); + ltsReadBlock(lt->tapeSet, datablocknum, thisbuf); if (!lt->frozen) ltsReleaseBlock(lt->tapeSet, datablocknum); lt->curBlockNumber = lt->nextBlockNumber; @@ -806,7 +806,7 @@ LogicalTapeWrite(LogicalTape *lt, void *ptr, size_t size) /* set the next-pointer and dump the current block. */ TapeBlockGetTrailer(lt->buffer)->next = nextBlockNumber; - ltsWriteBlock(lt->tapeSet, lt->curBlockNumber, (void *) lt->buffer); + ltsWriteBlock(lt->tapeSet, lt->curBlockNumber, lt->buffer); /* initialize the prev-pointer of the next block */ TapeBlockGetTrailer(lt->buffer)->prev = lt->curBlockNumber; @@ -826,7 +826,7 @@ LogicalTapeWrite(LogicalTape *lt, void *ptr, size_t size) lt->pos += nthistime; if (lt->nbytes < lt->pos) lt->nbytes = lt->pos; - ptr = (void *) ((char *) ptr + nthistime); + ptr = (char *) ptr + nthistime; size -= nthistime; } } @@ -888,7 +888,7 @@ LogicalTapeRewindForRead(LogicalTape *lt, size_t buffer_size) lt->buffer_size - lt->nbytes); TapeBlockSetNBytes(lt->buffer, lt->nbytes); - ltsWriteBlock(lt->tapeSet, lt->curBlockNumber, (void *) lt->buffer); + ltsWriteBlock(lt->tapeSet, lt->curBlockNumber, lt->buffer); } lt->writing = false; } @@ -953,7 +953,7 @@ LogicalTapeRead(LogicalTape *lt, void *ptr, size_t size) memcpy(ptr, lt->buffer + lt->pos, nthistime); lt->pos += nthistime; - ptr = (void *) ((char *) ptr + nthistime); + ptr = (char *) ptr + nthistime; size -= nthistime; nread += nthistime; } @@ -1004,7 +1004,7 @@ LogicalTapeFreeze(LogicalTape *lt, TapeShare *share) lt->buffer_size - lt->nbytes); TapeBlockSetNBytes(lt->buffer, lt->nbytes); - ltsWriteBlock(lt->tapeSet, lt->curBlockNumber, (void *) lt->buffer); + ltsWriteBlock(lt->tapeSet, lt->curBlockNumber, lt->buffer); } lt->writing = false; lt->frozen = true; @@ -1031,7 +1031,7 @@ LogicalTapeFreeze(LogicalTape *lt, TapeShare *share) if (lt->firstBlockNumber == -1L) lt->nextBlockNumber = -1L; - ltsReadBlock(lt->tapeSet, lt->curBlockNumber, (void *) lt->buffer); + ltsReadBlock(lt->tapeSet, lt->curBlockNumber, lt->buffer); if (TapeBlockIsLast(lt->buffer)) lt->nextBlockNumber = -1L; else @@ -1098,7 +1098,7 @@ LogicalTapeBackspace(LogicalTape *lt, size_t size) return seekpos; } - ltsReadBlock(lt->tapeSet, prev, (void *) lt->buffer); + ltsReadBlock(lt->tapeSet, prev, lt->buffer); if (TapeBlockGetTrailer(lt->buffer)->next != lt->curBlockNumber) elog(ERROR, "broken tape, next of block %ld is %ld, expected %ld", @@ -1142,7 +1142,7 @@ LogicalTapeSeek(LogicalTape *lt, long blocknum, int offset) if (blocknum != lt->curBlockNumber) { - ltsReadBlock(lt->tapeSet, blocknum, (void *) lt->buffer); + ltsReadBlock(lt->tapeSet, blocknum, lt->buffer); lt->curBlockNumber = blocknum; lt->nbytes = TapeBlockPayloadSize; lt->nextBlockNumber = TapeBlockGetTrailer(lt->buffer)->next; diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index 416f02ba3c..dd5fba0e36 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -2908,7 +2908,7 @@ markrunend(LogicalTape *tape) { unsigned int len = 0; - LogicalTapeWrite(tape, (void *) &len, sizeof(len)); + LogicalTapeWrite(tape, &len, sizeof(len)); } /* diff --git a/src/backend/utils/sort/tuplesortvariants.c b/src/backend/utils/sort/tuplesortvariants.c index dfe132660a..12ca6d7d75 100644 --- a/src/backend/utils/sort/tuplesortvariants.c +++ b/src/backend/utils/sort/tuplesortvariants.c @@ -1002,10 +1002,10 @@ writetup_heap(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup) /* total on-disk footprint: */ unsigned int tuplen = tupbodylen + sizeof(int); - LogicalTapeWrite(tape, (void *) &tuplen, sizeof(tuplen)); - LogicalTapeWrite(tape, (void *) tupbody, tupbodylen); + LogicalTapeWrite(tape, &tuplen, sizeof(tuplen)); + LogicalTapeWrite(tape, tupbody, tupbodylen); if (base->sortopt & TUPLESORT_RANDOMACCESS) /* need trailing length word? */ - LogicalTapeWrite(tape, (void *) &tuplen, sizeof(tuplen)); + LogicalTapeWrite(tape, &tuplen, sizeof(tuplen)); } static void @@ -1475,10 +1475,10 @@ writetup_index(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup) unsigned int tuplen; tuplen = IndexTupleSize(tuple) + sizeof(tuplen); - LogicalTapeWrite(tape, (void *) &tuplen, sizeof(tuplen)); - LogicalTapeWrite(tape, (void *) tuple, IndexTupleSize(tuple)); + LogicalTapeWrite(tape, &tuplen, sizeof(tuplen)); + LogicalTapeWrite(tape, tuple, IndexTupleSize(tuple)); if (base->sortopt & TUPLESORT_RANDOMACCESS) /* need trailing length word? */ - LogicalTapeWrite(tape, (void *) &tuplen, sizeof(tuplen)); + LogicalTapeWrite(tape, &tuplen, sizeof(tuplen)); } static void @@ -1564,10 +1564,10 @@ writetup_datum(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup) writtenlen = tuplen + sizeof(unsigned int); - LogicalTapeWrite(tape, (void *) &writtenlen, sizeof(writtenlen)); + LogicalTapeWrite(tape, &writtenlen, sizeof(writtenlen)); LogicalTapeWrite(tape, waddr, tuplen); if (base->sortopt & TUPLESORT_RANDOMACCESS) /* need trailing length word? */ - LogicalTapeWrite(tape, (void *) &writtenlen, sizeof(writtenlen)); + LogicalTapeWrite(tape, &writtenlen, sizeof(writtenlen)); } static void base-commit: f4f2f2b84a0bbf9edbfc4a8684040a941cd6d085 -- 2.39.0
From 3fec92a1eaff51f639a8b5f9de7970ef2af03a0b Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Fri, 23 Dec 2022 08:42:08 +0100 Subject: [PATCH 2/2] Add const to BufFileWrite Make data buffer argument to BufFileWrite a const pointer and bubble this up to various callers and related APIs. This makes the APIs clearer and more consistent. --- src/backend/access/gist/gistbuildbuffers.c | 4 ++-- src/backend/backup/backup_manifest.c | 4 ++-- src/backend/storage/file/buffile.c | 4 ++-- src/backend/utils/sort/logtape.c | 8 ++++---- src/include/storage/buffile.h | 2 +- src/include/utils/logtape.h | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/backend/access/gist/gistbuildbuffers.c b/src/backend/access/gist/gistbuildbuffers.c index 538e3880c9..60911e6aac 100644 --- a/src/backend/access/gist/gistbuildbuffers.c +++ b/src/backend/access/gist/gistbuildbuffers.c @@ -38,7 +38,7 @@ static long gistBuffersGetFreeBlock(GISTBuildBuffers *gfbb); static void gistBuffersReleaseBlock(GISTBuildBuffers *gfbb, long blocknum); static void ReadTempFileBlock(BufFile *file, long blknum, void *ptr); -static void WriteTempFileBlock(BufFile *file, long blknum, void *ptr); +static void WriteTempFileBlock(BufFile *file, long blknum, const void *ptr); /* @@ -764,7 +764,7 @@ ReadTempFileBlock(BufFile *file, long blknum, void *ptr) } static void -WriteTempFileBlock(BufFile *file, long blknum, void *ptr) +WriteTempFileBlock(BufFile *file, long blknum, const void *ptr) { if (BufFileSeekBlock(file, blknum) != 0) elog(ERROR, "could not seek to block %ld in temporary file", blknum); diff --git a/src/backend/backup/backup_manifest.c b/src/backend/backup/backup_manifest.c index a54185fdab..68a2337a21 100644 --- a/src/backend/backup/backup_manifest.c +++ b/src/backend/backup/backup_manifest.c @@ -21,7 +21,7 @@ #include "utils/builtins.h" #include "utils/json.h" -static void AppendStringToManifest(backup_manifest_info *manifest, char *s); +static void AppendStringToManifest(backup_manifest_info *manifest, const char *s); /* * Does the user want a backup manifest? @@ -385,7 +385,7 @@ SendBackupManifest(backup_manifest_info *manifest, bbsink *sink) * Append a cstring to the manifest. */ static void -AppendStringToManifest(backup_manifest_info *manifest, char *s) +AppendStringToManifest(backup_manifest_info *manifest, const char *s) { int len = strlen(s); diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c index b07cca28d6..2202ba43f7 100644 --- a/src/backend/storage/file/buffile.c +++ b/src/backend/storage/file/buffile.c @@ -622,7 +622,7 @@ BufFileRead(BufFile *file, void *ptr, size_t size) * ereport(). */ void -BufFileWrite(BufFile *file, void *ptr, size_t size) +BufFileWrite(BufFile *file, const void *ptr, size_t size) { size_t nthistime; @@ -655,7 +655,7 @@ BufFileWrite(BufFile *file, void *ptr, size_t size) file->pos += nthistime; if (file->nbytes < file->pos) file->nbytes = file->pos; - ptr = (char *) ptr + nthistime; + ptr = (const char *) ptr + nthistime; size -= nthistime; } } diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c index 9db220b7ea..23a5e7f2a0 100644 --- a/src/backend/utils/sort/logtape.c +++ b/src/backend/utils/sort/logtape.c @@ -220,7 +220,7 @@ struct LogicalTapeSet }; static LogicalTape *ltsCreateTape(LogicalTapeSet *lts); -static void ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer); +static void ltsWriteBlock(LogicalTapeSet *lts, long blocknum, const void *buffer); static void ltsReadBlock(LogicalTapeSet *lts, long blocknum, void *buffer); static long ltsGetBlock(LogicalTapeSet *lts, LogicalTape *lt); static long ltsGetFreeBlock(LogicalTapeSet *lts); @@ -235,7 +235,7 @@ static void ltsInitReadBuffer(LogicalTape *lt); * No need for an error return convention; we ereport() on any error. */ static void -ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer) +ltsWriteBlock(LogicalTapeSet *lts, long blocknum, const void *buffer) { /* * BufFile does not support "holes", so if we're about to write a block @@ -759,7 +759,7 @@ LogicalTapeSetForgetFreeSpace(LogicalTapeSet *lts) * There are no error returns; we ereport() on failure. */ void -LogicalTapeWrite(LogicalTape *lt, void *ptr, size_t size) +LogicalTapeWrite(LogicalTape *lt, const void *ptr, size_t size) { LogicalTapeSet *lts = lt->tapeSet; size_t nthistime; @@ -826,7 +826,7 @@ LogicalTapeWrite(LogicalTape *lt, void *ptr, size_t size) lt->pos += nthistime; if (lt->nbytes < lt->pos) lt->nbytes = lt->pos; - ptr = (char *) ptr + nthistime; + ptr = (const char *) ptr + nthistime; size -= nthistime; } } diff --git a/src/include/storage/buffile.h b/src/include/storage/buffile.h index a4922d1853..636a0b9071 100644 --- a/src/include/storage/buffile.h +++ b/src/include/storage/buffile.h @@ -39,7 +39,7 @@ typedef struct BufFile BufFile; extern BufFile *BufFileCreateTemp(bool interXact); extern void BufFileClose(BufFile *file); extern size_t BufFileRead(BufFile *file, void *ptr, size_t size); -extern void BufFileWrite(BufFile *file, void *ptr, size_t size); +extern void BufFileWrite(BufFile *file, const void *ptr, size_t size); extern int BufFileSeek(BufFile *file, int fileno, off_t offset, int whence); extern void BufFileTell(BufFile *file, int *fileno, off_t *offset); extern int BufFileSeekBlock(BufFile *file, long blknum); diff --git a/src/include/utils/logtape.h b/src/include/utils/logtape.h index 8c742ac491..1afd527e13 100644 --- a/src/include/utils/logtape.h +++ b/src/include/utils/logtape.h @@ -66,7 +66,7 @@ extern LogicalTape *LogicalTapeCreate(LogicalTapeSet *lts); extern LogicalTape *LogicalTapeImport(LogicalTapeSet *lts, int worker, TapeShare *shared); extern void LogicalTapeSetForgetFreeSpace(LogicalTapeSet *lts); extern size_t LogicalTapeRead(LogicalTape *lt, void *ptr, size_t size); -extern void LogicalTapeWrite(LogicalTape *lt, void *ptr, size_t size); +extern void LogicalTapeWrite(LogicalTape *lt, const void *ptr, size_t size); extern void LogicalTapeRewindForRead(LogicalTape *lt, size_t buffer_size); extern void LogicalTapeFreeze(LogicalTape *lt, TapeShare *share); extern size_t LogicalTapeBackspace(LogicalTape *lt, size_t size); -- 2.39.0