1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-07-07 00:35:37 +02:00

Clean up const usage in bufPtr() and bufRemainsPtr().

These functions accepted const Buffer objects and returned non-const pointers which is definitely not a good idea. Add bufPtrConst() to handle cases where only a const return value is needed and update call sites.

Use UNCONSTIFY() in cases where library code out of our control requires a non-const pointer. This includes the already-documented exception in command/backup/pageChecksum and input buffers in the gzCompress and gzDecompress filters.
This commit is contained in:
David Steele
2020-04-02 17:25:49 -04:00
parent 76b88a3cd5
commit 713211d89f
17 changed files with 63 additions and 36 deletions

View File

@ -99,7 +99,7 @@ pageChecksumProcess(THIS_VOID, const Buffer *input)
// being passed a const buffer and we should deinitely not be modifying the contents. When pgPageChecksumTest() returns // being passed a const buffer and we should deinitely not be modifying the contents. When pgPageChecksumTest() returns
// the data should be the same, but there's no question that some munging occurs. Should we make a copy of the page // the data should be the same, but there's no question that some munging occurs. Should we make a copy of the page
// before passing it into pgPageChecksumTest()? // before passing it into pgPageChecksumTest()?
unsigned char *pagePtr = bufPtr(input) + (pageIdx * PG_PAGE_SIZE_DEFAULT); unsigned char *pagePtr = UNCONSTIFY(unsigned char *, bufPtrConst(input)) + (pageIdx * PG_PAGE_SIZE_DEFAULT);
// Get a pointer to the page header at the beginning of the page // Get a pointer to the page header at the beginning of the page
const PageHeaderData *pageHeader = (const PageHeaderData *)pagePtr; const PageHeaderData *pageHeader = (const PageHeaderData *)pagePtr;

View File

@ -98,7 +98,9 @@ gzCompressProcess(THIS_VOID, const Buffer *uncompressed, Buffer *compressed)
if (!this->inputSame) if (!this->inputSame)
{ {
this->stream.avail_in = (unsigned int)bufUsed(uncompressed); this->stream.avail_in = (unsigned int)bufUsed(uncompressed);
this->stream.next_in = bufPtr(uncompressed);
// Not all versions of zlib (and none by default) will accept const input buffers
this->stream.next_in = UNCONSTIFY(unsigned char *, bufPtrConst(uncompressed));
} }
} }

View File

@ -85,7 +85,9 @@ gzDecompressProcess(THIS_VOID, const Buffer *compressed, Buffer *uncompressed)
if (!this->inputSame) if (!this->inputSame)
{ {
this->stream.avail_in = (unsigned int)bufUsed(compressed); this->stream.avail_in = (unsigned int)bufUsed(compressed);
this->stream.next_in = bufPtr(compressed);
// Not all versions of zlib (and none by default) will accept const input buffers
this->stream.next_in = UNCONSTIFY(unsigned char *, bufPtrConst(compressed));
} }
this->stream.avail_out = (unsigned int)bufRemains(uncompressed); this->stream.avail_out = (unsigned int)bufRemains(uncompressed);

View File

@ -183,7 +183,7 @@ lz4CompressProcess(THIS_VOID, const Buffer *uncompressed, Buffer *compressed)
output, output,
lz4Error( lz4Error(
LZ4F_compressUpdate( LZ4F_compressUpdate(
this->context, bufRemainsPtr(output), bufRemains(output), bufPtr(uncompressed), bufUsed(uncompressed), this->context, bufRemainsPtr(output), bufRemains(output), bufPtrConst(uncompressed), bufUsed(uncompressed),
NULL))); NULL)));
} }
// Else flush remaining output // Else flush remaining output

View File

@ -99,7 +99,8 @@ lz4DecompressProcess(THIS_VOID, const Buffer *compressed, Buffer *decompressed)
this->frameDone = lz4Error( this->frameDone = lz4Error(
LZ4F_decompress( LZ4F_decompress(
this->context, bufRemainsPtr(decompressed), &dstSize, bufPtr(compressed) + this->inputOffset, &srcSize, NULL)) == 0; this->context, bufRemainsPtr(decompressed), &dstSize, bufPtrConst(compressed) + this->inputOffset, &srcSize,
NULL)) == 0;
bufUsedInc(decompressed, dstSize); bufUsedInc(decompressed, dstSize);

View File

@ -331,7 +331,10 @@ cipherBlockProcess(THIS_VOID, const Buffer *source, Buffer *destination)
this->done = true; this->done = true;
} }
else else
destinationSizeActual = cipherBlockProcessBlock(this, bufPtr(source), bufUsed(source), bufRemainsPtr(outputActual)); {
destinationSizeActual = cipherBlockProcessBlock(
this, bufPtrConst(source), bufUsed(source), bufRemainsPtr(outputActual));
}
bufUsedInc(outputActual, destinationSizeActual); bufUsedInc(outputActual, destinationSizeActual);
@ -432,7 +435,7 @@ cipherBlockNew(CipherMode mode, CipherType cipherType, const Buffer *pass, const
// Store the passphrase // Store the passphrase
driver->pass = memNew(driver->passSize); driver->pass = memNew(driver->passSize);
memcpy(driver->pass, bufPtr(pass), driver->passSize); memcpy(driver->pass, bufPtrConst(pass), driver->passSize);
// Create param list // Create param list
VariantList *paramList = varLstNew(); VariantList *paramList = varLstNew();

View File

@ -84,7 +84,7 @@ cryptoHashProcess(THIS_VOID, const Buffer *message)
ASSERT(this->hash == NULL); ASSERT(this->hash == NULL);
ASSERT(message != NULL); ASSERT(message != NULL);
cryptoError(!EVP_DigestUpdate(this->hashContext, bufPtr(message), bufUsed(message)), "unable to process message hash"); cryptoError(!EVP_DigestUpdate(this->hashContext, bufPtrConst(message), bufUsed(message)), "unable to process message hash");
FUNCTION_LOG_RETURN_VOID(); FUNCTION_LOG_RETURN_VOID();
} }
@ -251,7 +251,7 @@ cryptoHmacOne(const String *type, const Buffer *key, const Buffer *message)
bufUsedSet(result, bufSize(result)); bufUsedSet(result, bufSize(result));
// Calculate the HMAC // Calculate the HMAC
HMAC(hashType, bufPtr(key), (int)bufUsed(key), bufPtr(message), bufUsed(message), bufPtr(result), NULL); HMAC(hashType, bufPtrConst(key), (int)bufUsed(key), bufPtrConst(message), bufUsed(message), bufPtr(result), NULL);
FUNCTION_LOG_RETURN(BUFFER, result); FUNCTION_LOG_RETURN(BUFFER, result);
} }

View File

@ -47,7 +47,8 @@ ioHandleWrite(THIS_VOID, const Buffer *buffer)
ASSERT(buffer != NULL); ASSERT(buffer != NULL);
THROW_ON_SYS_ERROR_FMT( THROW_ON_SYS_ERROR_FMT(
write(this->handle, bufPtr(buffer), bufUsed(buffer)) == -1, FileWriteError, "unable to write to %s", strPtr(this->name)); write(this->handle, bufPtrConst(buffer), bufUsed(buffer)) == -1, FileWriteError,
"unable to write to %s", strPtr(this->name));
FUNCTION_LOG_RETURN_VOID(); FUNCTION_LOG_RETURN_VOID();
} }

View File

@ -430,7 +430,7 @@ tlsClientWrite(THIS_VOID, const Buffer *buffer)
while (tlsWriteContinue(this, result, error, bufUsed(buffer))) while (tlsWriteContinue(this, result, error, bufUsed(buffer)))
{ {
result = SSL_write(this->session, bufPtr(buffer), (int)bufUsed(buffer)); result = SSL_write(this->session, bufPtrConst(buffer), (int)bufUsed(buffer));
error = SSL_get_error(this->session, result); error = SSL_get_error(this->session, result);
} }

View File

@ -360,11 +360,21 @@ bufLimitSet(Buffer *this, size_t limit)
FUNCTION_TEST_RETURN_VOID(); FUNCTION_TEST_RETURN_VOID();
} }
/*********************************************************************************************************************************** /**********************************************************************************************************************************/
Return buffer ptr
***********************************************************************************************************************************/
unsigned char * unsigned char *
bufPtr(const Buffer *this) bufPtr(Buffer *this)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(BUFFER, this);
FUNCTION_TEST_END();
ASSERT(this != NULL);
FUNCTION_TEST_RETURN(this->buffer);
}
const unsigned char *
bufPtrConst(const Buffer *this)
{ {
FUNCTION_TEST_BEGIN(); FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(BUFFER, this); FUNCTION_TEST_PARAM(BUFFER, this);
@ -390,11 +400,9 @@ bufRemains(const Buffer *this)
FUNCTION_TEST_RETURN(bufSize(this) - this->used); FUNCTION_TEST_RETURN(bufSize(this) - this->used);
} }
/*********************************************************************************************************************************** /**********************************************************************************************************************************/
Return pointer to remaining space in the buffer (after used space)
***********************************************************************************************************************************/
unsigned char * unsigned char *
bufRemainsPtr(const Buffer *this) bufRemainsPtr(Buffer *this)
{ {
FUNCTION_TEST_BEGIN(); FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(BUFFER, this); FUNCTION_TEST_PARAM(BUFFER, this);

View File

@ -35,9 +35,7 @@ Buffer *bufResize(Buffer *this, size_t size);
bool bufFull(const Buffer *this); bool bufFull(const Buffer *this);
void bufLimitClear(Buffer *this); void bufLimitClear(Buffer *this);
void bufLimitSet(Buffer *this, size_t limit); void bufLimitSet(Buffer *this, size_t limit);
unsigned char *bufPtr(const Buffer *this);
size_t bufRemains(const Buffer *this); size_t bufRemains(const Buffer *this);
unsigned char *bufRemainsPtr(const Buffer *this);
size_t bufSize(const Buffer *this); size_t bufSize(const Buffer *this);
size_t bufUsed(const Buffer *this); size_t bufUsed(const Buffer *this);
void bufUsedInc(Buffer *this, size_t inc); void bufUsedInc(Buffer *this, size_t inc);
@ -46,6 +44,18 @@ void bufUsedZero(Buffer *this);
void bufFree(Buffer *this); void bufFree(Buffer *this);
/***********************************************************************************************************************************
Getters
***********************************************************************************************************************************/
// Buffer pointer
unsigned char *bufPtr(Buffer *this);
// Const buffer pointer
const unsigned char *bufPtrConst(const Buffer *this);
// Pointer to remaining buffer space (after used space)
unsigned char *bufRemainsPtr(Buffer *this);
/*********************************************************************************************************************************** /***********************************************************************************************************************************
Fields that are common between dynamically allocated and constant buffers Fields that are common between dynamically allocated and constant buffers

View File

@ -123,7 +123,7 @@ strNewBuf(const Buffer *buffer)
// Allocate and assign string // Allocate and assign string
this->buffer = memNew(this->size + 1); this->buffer = memNew(this->size + 1);
memcpy(this->buffer, (char *)bufPtr(buffer), this->size); memcpy(this->buffer, bufPtrConst(buffer), this->size);
this->buffer[this->size] = 0; this->buffer[this->size] = 0;
FUNCTION_TEST_RETURN(this); FUNCTION_TEST_RETURN(this);

View File

@ -486,7 +486,7 @@ xmlDocumentNewBuf(const Buffer *buffer)
ASSERT(buffer != NULL); ASSERT(buffer != NULL);
ASSERT(bufSize(buffer) > 0); ASSERT(bufSize(buffer) > 0);
FUNCTION_TEST_RETURN(xmlDocumentNewC(bufPtr(buffer), bufUsed(buffer))); FUNCTION_TEST_RETURN(xmlDocumentNewC(bufPtrConst(buffer), bufUsed(buffer)));
} }
/*********************************************************************************************************************************** /***********************************************************************************************************************************

View File

@ -388,7 +388,7 @@ pgControlFromBuffer(const Buffer *controlFile)
for (unsigned int interfaceIdx = 0; interfaceIdx < PG_INTERFACE_SIZE; interfaceIdx++) for (unsigned int interfaceIdx = 0; interfaceIdx < PG_INTERFACE_SIZE; interfaceIdx++)
{ {
if (pgInterface[interfaceIdx].controlIs(bufPtr(controlFile))) if (pgInterface[interfaceIdx].controlIs(bufPtrConst(controlFile)))
{ {
interface = &pgInterface[interfaceIdx]; interface = &pgInterface[interfaceIdx];
break; break;
@ -398,7 +398,7 @@ pgControlFromBuffer(const Buffer *controlFile)
// If the version was not found then error with the control and catalog version that were found // If the version was not found then error with the control and catalog version that were found
if (interface == NULL) if (interface == NULL)
{ {
PgControlCommon *controlCommon = (PgControlCommon *)bufPtr(controlFile); const PgControlCommon *controlCommon = (const PgControlCommon *)bufPtrConst(controlFile);
THROW_FMT( THROW_FMT(
VersionNotSupportedError, VersionNotSupportedError,
@ -408,7 +408,7 @@ pgControlFromBuffer(const Buffer *controlFile)
} }
// Get info from the control file // Get info from the control file
PgControl result = interface->control(bufPtr(controlFile)); PgControl result = interface->control(bufPtrConst(controlFile));
result.version = interface->version; result.version = interface->version;
// Check the segment size // Check the segment size
@ -491,7 +491,7 @@ pgWalFromBuffer(const Buffer *walBuffer)
ASSERT(walBuffer != NULL); ASSERT(walBuffer != NULL);
// Check that this is a long format WAL header // Check that this is a long format WAL header
if (!(((PgWalCommon *)bufPtr(walBuffer))->flag & PG_WAL_LONG_HEADER)) if (!(((const PgWalCommon *)bufPtrConst(walBuffer))->flag & PG_WAL_LONG_HEADER))
THROW_FMT(FormatError, "first page header in WAL file is expected to be in long format"); THROW_FMT(FormatError, "first page header in WAL file is expected to be in long format");
// Search for the version of PostgreSQL that uses this WAL magic // Search for the version of PostgreSQL that uses this WAL magic
@ -499,7 +499,7 @@ pgWalFromBuffer(const Buffer *walBuffer)
for (unsigned int interfaceIdx = 0; interfaceIdx < PG_INTERFACE_SIZE; interfaceIdx++) for (unsigned int interfaceIdx = 0; interfaceIdx < PG_INTERFACE_SIZE; interfaceIdx++)
{ {
if (pgInterface[interfaceIdx].walIs(bufPtr(walBuffer))) if (pgInterface[interfaceIdx].walIs(bufPtrConst(walBuffer)))
{ {
interface = &pgInterface[interfaceIdx]; interface = &pgInterface[interfaceIdx];
break; break;
@ -513,11 +513,11 @@ pgWalFromBuffer(const Buffer *walBuffer)
VersionNotSupportedError, VersionNotSupportedError,
"unexpected WAL magic %u\n" "unexpected WAL magic %u\n"
"HINT: is this version of PostgreSQL supported?", "HINT: is this version of PostgreSQL supported?",
((PgWalCommon *)bufPtr(walBuffer))->magic); ((const PgWalCommon *)bufPtrConst(walBuffer))->magic);
} }
// Get info from the control file // Get info from the control file
PgWal result = interface->wal(bufPtr(walBuffer)); PgWal result = interface->wal(bufPtrConst(walBuffer));
result.version = interface->version; result.version = interface->version;
FUNCTION_LOG_RETURN(PG_WAL, result); FUNCTION_LOG_RETURN(PG_WAL, result);

View File

@ -139,7 +139,7 @@ storageWritePosix(THIS_VOID, const Buffer *buffer)
ASSERT(this->handle != -1); ASSERT(this->handle != -1);
// Write the data // Write the data
if (write(this->handle, bufPtr(buffer), bufUsed(buffer)) != (ssize_t)bufUsed(buffer)) if (write(this->handle, bufPtrConst(buffer), bufUsed(buffer)) != (ssize_t)bufUsed(buffer))
THROW_SYS_ERROR_FMT(FileWriteError, "unable to write '%s'", strPtr(this->nameTmp)); THROW_SYS_ERROR_FMT(FileWriteError, "unable to write '%s'", strPtr(this->nameTmp));
FUNCTION_LOG_RETURN_VOID(); FUNCTION_LOG_RETURN_VOID();

View File

@ -171,7 +171,7 @@ ioTestFilterMultiplyProcess(THIS_VOID, const Buffer *input, Buffer *output)
if (this->multiplyBuffer == NULL) if (this->multiplyBuffer == NULL)
{ {
this->multiplyBuffer = bufNew(bufUsed(input) * this->multiplier); this->multiplyBuffer = bufNew(bufUsed(input) * this->multiplier);
unsigned char *inputPtr = bufPtr(input); const unsigned char *inputPtr = bufPtrConst(input);
unsigned char *bufferPtr = bufPtr(this->multiplyBuffer); unsigned char *bufferPtr = bufPtr(this->multiplyBuffer);
for (unsigned int charIdx = 0; charIdx < bufUsed(input); charIdx++) for (unsigned int charIdx = 0; charIdx < bufUsed(input); charIdx++)

View File

@ -793,13 +793,13 @@ testRun(void)
// ------------------------------------------------------------------------------------------------------------------------- // -------------------------------------------------------------------------------------------------------------------------
TEST_ASSIGN(buffer, storageGetP(storageNewReadP(storageTest, strNewFmt("%s/test.txt", testPath()))), "get text"); TEST_ASSIGN(buffer, storageGetP(storageNewReadP(storageTest, strNewFmt("%s/test.txt", testPath()))), "get text");
TEST_RESULT_UINT(bufSize(buffer), 9, "check size"); TEST_RESULT_UINT(bufSize(buffer), 9, "check size");
TEST_RESULT_BOOL(memcmp(bufPtr(buffer), "TESTFILE\n", bufSize(buffer)) == 0, true, "check content"); TEST_RESULT_BOOL(memcmp(bufPtrConst(buffer), "TESTFILE\n", bufSize(buffer)) == 0, true, "check content");
// ------------------------------------------------------------------------------------------------------------------------- // -------------------------------------------------------------------------------------------------------------------------
TEST_ASSIGN( TEST_ASSIGN(
buffer, storageGetP(storageNewReadP(storageTest, strNewFmt("%s/test.txt", testPath())), .exactSize = 4), "get exact"); buffer, storageGetP(storageNewReadP(storageTest, strNewFmt("%s/test.txt", testPath())), .exactSize = 4), "get exact");
TEST_RESULT_UINT(bufSize(buffer), 4, "check size"); TEST_RESULT_UINT(bufSize(buffer), 4, "check size");
TEST_RESULT_BOOL(memcmp(bufPtr(buffer), "TEST", bufSize(buffer)) == 0, true, "check content"); TEST_RESULT_BOOL(memcmp(bufPtrConst(buffer), "TEST", bufSize(buffer)) == 0, true, "check content");
TEST_ERROR_FMT( TEST_ERROR_FMT(
storageGetP(storageNewReadP(storageTest, strNewFmt("%s/test.txt", testPath())), .exactSize = 64), FileReadError, storageGetP(storageNewReadP(storageTest, strNewFmt("%s/test.txt", testPath())), .exactSize = 64), FileReadError,
@ -810,7 +810,7 @@ testRun(void)
TEST_ASSIGN(buffer, storageGetP(storageNewReadP(storageTest, strNewFmt("%s/test.txt", testPath()))), "get text"); TEST_ASSIGN(buffer, storageGetP(storageNewReadP(storageTest, strNewFmt("%s/test.txt", testPath()))), "get text");
TEST_RESULT_UINT(bufSize(buffer), 9, "check size"); TEST_RESULT_UINT(bufSize(buffer), 9, "check size");
TEST_RESULT_BOOL(memcmp(bufPtr(buffer), "TESTFILE\n", bufSize(buffer)) == 0, true, "check content"); TEST_RESULT_BOOL(memcmp(bufPtrConst(buffer), "TESTFILE\n", bufSize(buffer)) == 0, true, "check content");
// ------------------------------------------------------------------------------------------------------------------------- // -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("read limited bytes"); TEST_TITLE("read limited bytes");
@ -820,7 +820,7 @@ testRun(void)
TEST_ASSIGN( TEST_ASSIGN(
buffer, storageGetP(storageNewReadP(storageTest, strNewFmt("%s/test.txt", testPath()), .limit = VARUINT64(7))), "get"); buffer, storageGetP(storageNewReadP(storageTest, strNewFmt("%s/test.txt", testPath()), .limit = VARUINT64(7))), "get");
TEST_RESULT_UINT(bufSize(buffer), 7, "check size"); TEST_RESULT_UINT(bufSize(buffer), 7, "check size");
TEST_RESULT_BOOL(memcmp(bufPtr(buffer), "TESTFIL", bufSize(buffer)) == 0, true, "check content"); TEST_RESULT_BOOL(memcmp(bufPtrConst(buffer), "TESTFIL", bufSize(buffer)) == 0, true, "check content");
} }
// ***************************************************************************************************************************** // *****************************************************************************************************************************