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 <[email protected]>
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 <[email protected]>
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
