From 31fd69bb611da2a129d83e3249839b3b74951250 Mon Sep 17 00:00:00 2001 From: Viktor Kurilko Date: Fri, 7 Feb 2025 15:17:16 +0700 Subject: [PATCH] review based fix --- src/common/walFilter/postgresCommon.h | 2 +- .../walFilter/versions/definitionsGPDB7.h | 6 +-- .../walFilter/versions/recordProcessGPDB7.c | 52 +++++++++---------- test/src/common/harnessWal/harnessGPDB7.c | 24 ++++----- test/src/module/common/walFilterGPDB7Test.c | 12 ++--- 5 files changed, 48 insertions(+), 48 deletions(-) diff --git a/src/common/walFilter/postgresCommon.h b/src/common/walFilter/postgresCommon.h index 337a813d87..babdbad9cf 100644 --- a/src/common/walFilter/postgresCommon.h +++ b/src/common/walFilter/postgresCommon.h @@ -95,7 +95,7 @@ typedef struct XLogLongPageHeaderData uint32 xlp_xlog_blcksz; /* just as a cross-check */ } XLogLongPageHeaderData; -// This part of XLogRecord is the same in all supported versions of Postgresql. +// This part of XLogRecord is the same in all supported versions of PostgreSQL. typedef struct XLogRecordBase { uint32 xl_tot_len; /* total len of entire record */ diff --git a/src/common/walFilter/versions/definitionsGPDB7.h b/src/common/walFilter/versions/definitionsGPDB7.h index 1fe9ea1824..fbcdf01165 100644 --- a/src/common/walFilter/versions/definitionsGPDB7.h +++ b/src/common/walFilter/versions/definitionsGPDB7.h @@ -67,11 +67,11 @@ typedef struct XLogRecordGPDB7 uint8 xl_info; /* flag bits, see below */ RmgrId xl_rmid; /* resource manager for this record */ /* 2 bytes of padding here, initialize to zero */ - uint32_t xl_crc; /* CRC for this record */ + uint32 xl_crc; /* CRC for this record */ /* XLogRecordBlockHeaders and XLogRecordDataHeader follow, no padding */ } XLogRecordGPDB7; -#define XLogRecordData(record) ((uint8_t *) record + sizeof(XLogRecordGPDB7)) +#define XLogRecordData(record) ((uint8 *) record + sizeof(XLogRecordGPDB7)) _Static_assert( offsetof(XLogRecordGPDB7, xl_info) / 8 == offsetof(XLogRecordGPDB7, xl_rmid) / 8, "The xl_info and xl_rmid fields are in different 8 byte chunks."); @@ -79,7 +79,7 @@ _Static_assert( FN_INLINE_ALWAYS pg_crc32 xLogRecordChecksumGPDB7(const XLogRecordGPDB7 *const record) { - uint32_t crc = crc32cInit(); + uint32 crc = crc32cInit(); /* Calculate the CRC */ crc = crc32cComp(crc, ((unsigned char *) record) + sizeof(XLogRecordGPDB7), record->xl_tot_len - sizeof(XLogRecordGPDB7)); diff --git a/src/common/walFilter/versions/recordProcessGPDB7.c b/src/common/walFilter/versions/recordProcessGPDB7.c index 8e487508a5..4d3056b27b 100644 --- a/src/common/walFilter/versions/recordProcessGPDB7.c +++ b/src/common/walFilter/versions/recordProcessGPDB7.c @@ -26,14 +26,14 @@ typedef uint16 RepOriginId; typedef struct XLogRecordDataHeaderShort { - uint8_t id; /* XLR_BLOCK_ID_DATA_SHORT */ - uint8_t data_length; /* number of payload bytes */ + uint8 id; /* XLR_BLOCK_ID_DATA_SHORT */ + uint8 data_length; /* number of payload bytes */ } __attribute__((packed)) XLogRecordDataHeaderShort; typedef struct XLogRecordDataHeaderLong { - uint8_t id; /* XLR_BLOCK_ID_DATA_LONG */ - uint32_t data_length; /* number of payload bytes */ + uint8 id; /* XLR_BLOCK_ID_DATA_LONG */ + uint32 data_length; /* number of payload bytes */ } __attribute__((packed)) XLogRecordDataHeaderLong; static void @@ -82,13 +82,13 @@ validXLogRecordGPDB7(const XLogRecordBase *const recordBase, __attribute__((unus } } -FN_EXTERN uint32_t +FN_EXTERN uint32 xLogRecordHeaderSizeGPDB7(void) { return sizeof(XLogRecordGPDB7); } -FN_EXTERN uint32_t +FN_EXTERN uint32 xLogRecordRmidSizeGPDB7(void) { return offsetof(XLogRecordGPDB7, xl_rmid) + SIZE_OF_STRUCT_MEMBER(XLogRecordGPDB7, xl_rmid); @@ -104,7 +104,7 @@ xLogRecordIsWalSwitchGPDB7(const XLogRecordBase *recordBase) static const RelFileNode * getRelFileNodeFromMainData(const XLogRecordGPDB7 *const record, const void *const mainData) { - uint8_t info = (uint8_t) (record->xl_info & ~XLR_INFO_MASK); + uint8 info = (uint8) (record->xl_info & ~XLR_INFO_MASK); switch (record->xl_rmid) { case RM7_SMGR_ID: @@ -120,7 +120,7 @@ getRelFileNodeFromMainData(const XLogRecordGPDB7 *const record, const void *cons } default: - THROW_FMT(FormatError, "unknown Storage record: %d", info); + THROW_FMT(FormatError, "unknown Storage record: %" PRIu8, info); } case RM7_HEAP2_ID: @@ -219,14 +219,14 @@ getRelFileNodes(XLogRecordGPDB7 *const record, PgPageSize pageSize) /* Decode the headers */ size_t datatotal = 0; int maxBlockId = -1; - const uint8_t *ptr = (uint8_t *) record; + const uint8 *ptr = (uint8 *) record; size_t offset = sizeof(XLogRecordGPDB7); RelFileNode *relFileNode = NULL; List *result = lstNewP(sizeof(RelFileNode)); - uint32_t mainDataSize = 0; - // Read only the headers. - // All headers are read when the total size of all backup blocks and main data is greater than the remaining data. + uint32 mainDataSize = 0; + // Read the headers only. + // The next header exists if the remaining data is greater than the total size of all backup blocks and main data. while (record->xl_tot_len - offset > datatotal) { uint8 block_id; @@ -234,7 +234,7 @@ getRelFileNodes(XLogRecordGPDB7 *const record, PgPageSize pageSize) if (block_id == XLR_BLOCK_ID_DATA_SHORT) { - uint8_t size; + uint8 size; COPY_HEADER_FIELD(size); mainDataSize = size; break; /* by convention, the main data fragment is @@ -257,7 +257,7 @@ getRelFileNodes(XLogRecordGPDB7 *const record, PgPageSize pageSize) } /* XLogRecordBlockHeader */ - uint8_t fork_flags; + uint8 fork_flags; if (block_id <= maxBlockId) { @@ -268,7 +268,7 @@ getRelFileNodes(XLogRecordGPDB7 *const record, PgPageSize pageSize) COPY_HEADER_FIELD(fork_flags); bool has_data = ((fork_flags & BKPBLOCK_HAS_DATA) != 0); - uint16_t data_len; + uint16 data_len; COPY_HEADER_FIELD(data_len); /* cross-check that the HAS_DATA flag is set iff data_length > 0 */ @@ -284,15 +284,15 @@ getRelFileNodes(XLogRecordGPDB7 *const record, PgPageSize pageSize) if (fork_flags & BKPBLOCK_HAS_IMAGE) { - uint16_t bimg_len; - uint16_t hole_offset; - uint8_t bimg_info; + uint16 bimg_len; + uint16 hole_offset; + uint8 bimg_info; COPY_HEADER_FIELD(bimg_len); COPY_HEADER_FIELD(hole_offset); COPY_HEADER_FIELD(bimg_info); - uint16_t hole_length; + uint16 hole_length; if (bimg_info & BKPIMAGE_IS_COMPRESSED) { if (bimg_info & BKPIMAGE_HAS_HOLE) @@ -301,7 +301,7 @@ getRelFileNodes(XLogRecordGPDB7 *const record, PgPageSize pageSize) hole_length = 0; } else - hole_length = (uint16_t) (pageSize - bimg_len); + hole_length = (uint16) (pageSize - bimg_len); datatotal += bimg_len; /* @@ -361,7 +361,7 @@ getRelFileNodes(XLogRecordGPDB7 *const record, PgPageSize pageSize) { ASSERT(record->xl_tot_len > mainDataSize); // Main data is always the last one - const void *const mainDataPtr = ((uint8_t *) record) + record->xl_tot_len - mainDataSize; + const void *const mainDataPtr = ((uint8 *) record) + record->xl_tot_len - mainDataSize; const RelFileNode *const mainDataRelFileNode = getRelFileNodeFromMainData(record, mainDataPtr); if (mainDataRelFileNode) lstAdd(result, mainDataRelFileNode); @@ -373,19 +373,19 @@ getRelFileNodes(XLogRecordGPDB7 *const record, PgPageSize pageSize) static void overrideXLogRecordBody(XLogRecordGPDB7 *const record) { - uint8_t *recordData = XLogRecordData(record); + uint8 *recordData = XLogRecordData(record); if (record->xl_tot_len - sizeof(XLogRecordGPDB7) - sizeof(XLogRecordDataHeaderShort) <= UINT8_MAX) { *((XLogRecordDataHeaderShort *) recordData) = (XLogRecordDataHeaderShort){ XLR_BLOCK_ID_DATA_SHORT, - (uint8_t) (record->xl_tot_len - sizeof(XLogRecordGPDB7) - sizeof(XLogRecordDataHeaderShort)) + (uint8) (record->xl_tot_len - sizeof(XLogRecordGPDB7) - sizeof(XLogRecordDataHeaderShort)) }; } else { *((XLogRecordDataHeaderLong *) recordData) = (XLogRecordDataHeaderLong){ XLR_BLOCK_ID_DATA_LONG, - (uint32_t) (record->xl_tot_len - sizeof(XLogRecordGPDB7) - sizeof(XLogRecordDataHeaderLong)) + (uint32) (record->xl_tot_len - sizeof(XLogRecordGPDB7) - sizeof(XLogRecordDataHeaderLong)) }; } } @@ -411,7 +411,7 @@ filterRecordGPDB7(XLogRecordBase *const recordBase, const PgPageSize pageSize) bool hasPassFilter = false; bool hasNotPassFilter = false; List *notPassFilterList = lstNewP(sizeof(RelFileNode)); - for (uint32_t i = 0; i < lstSize(nodes); i++) + for (uint32 i = 0; i < lstSize(nodes); i++) { const RelFileNode *const relFileNode = lstGet(nodes, i); if (isRelationNeeded(relFileNode->dbNode, relFileNode->spcNode, relFileNode->relNode)) @@ -436,7 +436,7 @@ filterRecordGPDB7(XLogRecordBase *const recordBase, const PgPageSize pageSize) ASSERT(!lstEmpty(notPassFilterList)); String *errMessage = strCatZ(strNew(), "The following RefFileNodes cannot be filtered out because they are in the same" " XLogRecord as the RefFileNode that passes the filter. ["); - for (uint32_t i = 0; i < lstSize(notPassFilterList); i++) + for (uint32 i = 0; i < lstSize(notPassFilterList); i++) { if (i > 0) strCatZ(errMessage, ", "); diff --git a/test/src/common/harnessWal/harnessGPDB7.c b/test/src/common/harnessWal/harnessGPDB7.c index 1f01b36b12..fc5dcc0a79 100644 --- a/test/src/common/harnessWal/harnessGPDB7.c +++ b/test/src/common/harnessWal/harnessGPDB7.c @@ -12,25 +12,25 @@ #define WRITE_FIELD_CONST(type, data) \ do { \ record = memResize(record, offset + sizeof(type)); \ - *((type *) ((uint8_t *) record + offset)) = data; \ + *((type *) ((uint8 *) record + offset)) = data; \ offset += sizeof(type); \ } while (0) #define WRITE_FIELD(data) \ WRITE_FIELD_CONST(__typeof__(data), data) -#define WRITE_DATA(data, size) \ - do { \ - record = memResize(record, offset + size); \ - if (data) \ - memcpy((uint8_t *) record + offset, data, size); \ - else \ - memset((uint8_t *) record + offset, RECORD_BODY_PLACEHOLDER, size); \ - offset += size; \ +#define WRITE_DATA(data, size) \ + do { \ + record = memResize(record, offset + size); \ + if (data) \ + memcpy((uint8 *) record + offset, data, size); \ + else \ + memset((uint8 *) record + offset, RECORD_BODY_PLACEHOLDER, size); \ + offset += size; \ } while (0) XLogRecordBase * -hrnGpdbCreateXRecord12GPDB(uint8_t rmid, uint8_t info, CreateXRecordParam param) +hrnGpdbCreateXRecord12GPDB(uint8 rmid, uint8 info, CreateXRecordParam param) { XLogRecordGPDB7 *record = memNew(sizeof(XLogRecordGPDB7)); *record = (XLogRecordGPDB7){ @@ -82,7 +82,7 @@ hrnGpdbCreateXRecord12GPDB(uint8_t rmid, uint8_t info, CreateXRecordParam param) { if (param.main_data_size <= UINT8_MAX) { - uint8_t bodySizeSmall = (uint8_t) param.main_data_size; + uint8 bodySizeSmall = (uint8_t) param.main_data_size; // block id WRITE_FIELD_CONST(uint8_t, XLR_BLOCK_ID_DATA_SHORT); WRITE_FIELD(bodySizeSmall); @@ -104,7 +104,7 @@ hrnGpdbCreateXRecord12GPDB(uint8_t rmid, uint8_t info, CreateXRecordParam param) if (block->fork_flags & BKPBLOCK_HAS_IMAGE) { record = memResize(record, offset + block->bimg_len); - memset((uint8_t *) record + offset, RECORD_BODY_PLACEHOLDER, block->bimg_len); + memset((uint8 *) record + offset, RECORD_BODY_PLACEHOLDER, block->bimg_len); offset += block->bimg_len; } diff --git a/test/src/module/common/walFilterGPDB7Test.c b/test/src/module/common/walFilterGPDB7Test.c index 4e3e6d0152..59c98d09e4 100644 --- a/test/src/module/common/walFilterGPDB7Test.c +++ b/test/src/module/common/walFilterGPDB7Test.c @@ -23,8 +23,8 @@ typedef enum WalFlags typedef struct XRecordInfo { - uint8_t rmid; - uint8_t info; + uint8 rmid; + uint8 info; RelFileNode node; bool main_data; } XRecordInfo; @@ -132,13 +132,13 @@ insertWalSwitchXRecord(Buffer *wal) } void -testGetRelfilenode(uint8_t rmid, uint8_t info, bool expect_not_skip) +testGetRelfilenode(uint8 rmid, uint8 info, bool expect_not_skip) { RelFileNode node = {1, 2, 3}; XLogRecordBase *record = createXRecord(rmid, info, .main_data_size = sizeof(node), .main_data = &node); - const RelFileNode *nodeResult = getRelFileNodeFromMainData((XLogRecordGPDB7 *) record, (uint8_t *) record + sizeof(XLogRecordGPDB7) + 2); + const RelFileNode *nodeResult = getRelFileNodeFromMainData((XLogRecordGPDB7 *) record, (uint8 *) record + sizeof(XLogRecordGPDB7) + 2); RelFileNode nodeExpect = {1, 2, 3}; TEST_RESULT_BOOL(nodeResult != NULL, expect_not_skip, "RelFileNode is different from expected"); if (expect_not_skip) @@ -1225,7 +1225,7 @@ testRun(void) .rnode = {1, 2, 3} }; record = createXRecord(RM7_SMGR_ID, XLOG_SMGR_TRUNCATE, .main_data_size = sizeof(xlrec), .main_data = &xlrec); - const RelFileNode *nodeResult = getRelFileNodeFromMainData((XLogRecordGPDB7 *) record, (uint8_t *) record + sizeof(XLogRecordGPDB7) + 2); + const RelFileNode *nodeResult = getRelFileNodeFromMainData((XLogRecordGPDB7 *) record, (uint8 *) record + sizeof(XLogRecordGPDB7) + 2); TEST_RESULT_BOOL(nodeResult != NULL, true, "relFileNode not found"); TEST_RESULT_BOOL(memcmp(&xlrec.rnode, nodeResult, sizeof(RelFileNode)) == 0, true, "relFileNode is not the same"); } @@ -1242,7 +1242,7 @@ testRun(void) .target = {1, 2, 3} }; record = createXRecord(RM7_HEAP2_ID, XLOG_HEAP2_NEW_CID, .main_data_size = sizeof(xlrec), .main_data = &xlrec); - const RelFileNode *nodeResult = getRelFileNodeFromMainData((XLogRecordGPDB7 *) record, (uint8_t *) record + sizeof(XLogRecordGPDB7) + 2); + const RelFileNode *nodeResult = getRelFileNodeFromMainData((XLogRecordGPDB7 *) record, (uint8 *) record + sizeof(XLogRecordGPDB7) + 2); TEST_RESULT_BOOL(nodeResult != NULL, true, "relFileNode not found"); TEST_RESULT_BOOL(memcmp(&xlrec.target, nodeResult, sizeof(RelFileNode)) == 0, true, "relFileNode is not the same"); }