Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit b63bea5

Browse files
committed
Further improvements to c8f621c.
Coverity and inspection for the issue addressed in fd45d16 found some questionable code. Specifically coverity noticed that the wrong length was added in ReorderBufferSerializeChange() - without immediate negative consequences as the variable isn't used afterwards. During code-review and testing I noticed that a bit of space was wasted when allocating tuple bufs in several places. Thirdly, the debug memset()s in ReorderBufferGetTupleBuf() reduce the error checking valgrind can do. Backpatch: 9.4, like c8f621c.
1 parent 3fc6e2d commit b63bea5

File tree

2 files changed

+23
-10
lines changed

2 files changed

+23
-10
lines changed

src/backend/replication/logical/decode.c

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -607,13 +607,14 @@ DecodeInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
607607

608608
if (xlrec->flags & XLH_INSERT_CONTAINS_NEW_TUPLE)
609609
{
610-
Size tuplelen;
611-
char *tupledata = XLogRecGetBlockData(r, 0, &tuplelen);
610+
Size datalen;
611+
char *tupledata = XLogRecGetBlockData(r, 0, &datalen);
612+
Size tuplelen = datalen - SizeOfHeapHeader;
612613

613614
change->data.tp.newtuple =
614615
ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
615616

616-
DecodeXLogTuple(tupledata, tuplelen, change->data.tp.newtuple);
617+
DecodeXLogTuple(tupledata, datalen, change->data.tp.newtuple);
617618
}
618619

619620
change->data.tp.clear_toast_afterwards = true;
@@ -634,7 +635,6 @@ DecodeUpdate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
634635
xl_heap_update *xlrec;
635636
ReorderBufferChange *change;
636637
char *data;
637-
Size datalen;
638638
RelFileNode target_node;
639639

640640
xlrec = (xl_heap_update *) XLogRecGetData(r);
@@ -655,22 +655,31 @@ DecodeUpdate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
655655

656656
if (xlrec->flags & XLH_UPDATE_CONTAINS_NEW_TUPLE)
657657
{
658+
Size datalen;
659+
Size tuplelen;
660+
658661
data = XLogRecGetBlockData(r, 0, &datalen);
659662

663+
tuplelen = datalen - SizeOfHeapHeader;
664+
660665
change->data.tp.newtuple =
661-
ReorderBufferGetTupleBuf(ctx->reorder, datalen);
666+
ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
662667

663668
DecodeXLogTuple(data, datalen, change->data.tp.newtuple);
664669
}
665670

666671
if (xlrec->flags & XLH_UPDATE_CONTAINS_OLD)
667672
{
673+
Size datalen;
674+
Size tuplelen;
675+
668676
/* caution, remaining data in record is not aligned */
669677
data = XLogRecGetData(r) + SizeOfHeapUpdate;
670678
datalen = XLogRecGetDataLen(r) - SizeOfHeapUpdate;
679+
tuplelen = datalen - SizeOfHeapHeader;
671680

672681
change->data.tp.oldtuple =
673-
ReorderBufferGetTupleBuf(ctx->reorder, datalen);
682+
ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
674683

675684
DecodeXLogTuple(data, datalen, change->data.tp.oldtuple);
676685
}
@@ -720,15 +729,16 @@ DecodeDelete(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
720729
/* old primary key stored */
721730
if (xlrec->flags & XLH_DELETE_CONTAINS_OLD)
722731
{
723-
Size len = XLogRecGetDataLen(r) - SizeOfHeapDelete;
732+
Size datalen = XLogRecGetDataLen(r) - SizeOfHeapDelete;
733+
Size tuplelen = datalen - SizeOfHeapHeader;
724734

725735
Assert(XLogRecGetDataLen(r) > (SizeOfHeapDelete + SizeOfHeapHeader));
726736

727737
change->data.tp.oldtuple =
728-
ReorderBufferGetTupleBuf(ctx->reorder, len);
738+
ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
729739

730740
DecodeXLogTuple((char *) xlrec + SizeOfHeapDelete,
731-
len, change->data.tp.oldtuple);
741+
datalen, change->data.tp.oldtuple);
732742
}
733743

734744
change->data.tp.clear_toast_afterwards = true;

src/backend/replication/logical/reorderbuffer.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,12 +471,15 @@ ReorderBufferGetTupleBuf(ReorderBuffer *rb, Size tuple_len)
471471
rb->nr_cached_tuplebufs--;
472472
tuple = slist_container(ReorderBufferTupleBuf, node,
473473
slist_pop_head_node(&rb->cached_tuplebufs));
474+
Assert(tuple->alloc_tuple_size == MaxHeapTupleSize);
474475
#ifdef USE_ASSERT_CHECKING
475476
memset(&tuple->tuple, 0xa9, sizeof(HeapTupleData));
477+
VALGRIND_MAKE_MEM_UNDEFINED(&tuple->tuple, sizeof(HeapTupleData));
476478
#endif
477479
tuple->tuple.t_data = ReorderBufferTupleBufData(tuple);
478480
#ifdef USE_ASSERT_CHECKING
479481
memset(tuple->tuple.t_data, 0xa8, tuple->alloc_tuple_size);
482+
VALGRIND_MAKE_MEM_UNDEFINED(tuple->tuple.t_data, tuple->alloc_tuple_size);
480483
#endif
481484
}
482485
else
@@ -2152,7 +2155,7 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
21522155
data += sizeof(HeapTupleData);
21532156

21542157
memcpy(data, newtup->tuple.t_data, newlen);
2155-
data += oldlen;
2158+
data += newlen;
21562159
}
21572160
break;
21582161
}

0 commit comments

Comments
 (0)