Skip to content

Commit

Permalink
review based fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@KnightMurloc
KnightMurloc committed Feb 7, 2025
1 parent e811044 commit 31fd69b
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 48 deletions.
2 changes: 1 addition & 1 deletion src/common/walFilter/postgresCommon.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
6 changes: 3 additions & 3 deletions src/common/walFilter/versions/definitionsGPDB7.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,19 @@ 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.");

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));
Expand Down
52 changes: 26 additions & 26 deletions src/common/walFilter/versions/recordProcessGPDB7.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -219,22 +219,22 @@ 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;
COPY_HEADER_FIELD(block_id);

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
Expand All @@ -257,7 +257,7 @@ getRelFileNodes(XLogRecordGPDB7 *const record, PgPageSize pageSize)
}

/* XLogRecordBlockHeader */
uint8_t fork_flags;
uint8 fork_flags;

if (block_id <= maxBlockId)
{
Expand All @@ -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 */
Expand All @@ -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)
Expand All @@ -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;

/*
Expand Down Expand Up @@ -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);
Expand All @@ -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))
};
}
}
Expand All @@ -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))
Expand All @@ -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, ", ");
Expand Down
24 changes: 12 additions & 12 deletions test/src/common/harnessWal/harnessGPDB7.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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){
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down
12 changes: 6 additions & 6 deletions test/src/module/common/walFilterGPDB7Test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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");
}
Expand All @@ -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");
}
Expand Down

0 comments on commit 31fd69b

Please sign in to comment.