Further improvements to c8f621c.

anarazel · anarazel · commit e3e84fd35838 · 2016-03-07T14:24:58.000-08:00
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.
diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
@@ -625,15 +625,16 @@ DecodeInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 
 	if (xlrec->flags & XLOG_HEAP_CONTAINS_NEW_TUPLE)
 	{
-		Size		tuplelen = r->xl_len - SizeOfHeapInsert;
+		Size		datalen = r->xl_len - SizeOfHeapInsert;
+		Size		tuplelen = datalen - SizeOfHeapHeader;
 
 		Assert(r->xl_len > (SizeOfHeapInsert + SizeOfHeapHeader));
 
 		change->data.tp.newtuple =
 			ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
 
 		DecodeXLogTuple((char *) xlrec + SizeOfHeapInsert,
-						tuplelen, change->data.tp.newtuple);
+						datalen, change->data.tp.newtuple);
 	}
 
 	change->data.tp.clear_toast_afterwards = true;
@@ -670,6 +671,7 @@ DecodeUpdate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 
 	if (xlrec->flags & XLOG_HEAP_CONTAINS_NEW_TUPLE)
 	{
+		Size		datalen;
 		Size		tuplelen;
 		xl_heap_header_len xlhdr;
 
@@ -678,12 +680,13 @@ DecodeUpdate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 		memcpy(&xlhdr, data, sizeof(xlhdr));
 		data += offsetof(xl_heap_header_len, header);
 
-		tuplelen = xlhdr.t_len + SizeOfHeapHeader;
+		datalen = xlhdr.t_len + SizeOfHeapHeader;
+		tuplelen = xlhdr.t_len;
 
 		change->data.tp.newtuple =
 			ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
 
-		DecodeXLogTuple(data, tuplelen, change->data.tp.newtuple);
+		DecodeXLogTuple(data, datalen, change->data.tp.newtuple);
 		/* skip over the rest of the tuple header */
 		data += SizeOfHeapHeader;
 		/* skip over the tuple data */
@@ -692,18 +695,20 @@ DecodeUpdate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 
 	if (xlrec->flags & XLOG_HEAP_CONTAINS_OLD)
 	{
+		Size		datalen;
 		Size		tuplelen;
 		xl_heap_header_len xlhdr;
 
 		memcpy(&xlhdr, data, sizeof(xlhdr));
 		data += offsetof(xl_heap_header_len, header);
 
-		tuplelen = xlhdr.t_len + SizeOfHeapHeader;
+		datalen = xlhdr.t_len + SizeOfHeapHeader;
+		tuplelen = xlhdr.t_len;
 
 		change->data.tp.oldtuple =
 			ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
 
-		DecodeXLogTuple(data, tuplelen, change->data.tp.oldtuple);
+		DecodeXLogTuple(data, datalen, change->data.tp.oldtuple);
 #ifdef NOT_USED
 		data += SizeOfHeapHeader;
 		data += xlhdr.t_len;
@@ -741,15 +746,16 @@ DecodeDelete(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 	/* old primary key stored */
 	if (xlrec->flags & XLOG_HEAP_CONTAINS_OLD)
 	{
-		Size		len = r->xl_len - SizeOfHeapDelete;
+		Size		datalen = r->xl_len - SizeOfHeapDelete;
+		Size		tuplelen = datalen - SizeOfHeapHeader;
 
 		Assert(r->xl_len > (SizeOfHeapDelete + SizeOfHeapHeader));
 
 		change->data.tp.oldtuple =
-			ReorderBufferGetTupleBuf(ctx->reorder, len);
+			ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
 
 		DecodeXLogTuple((char *) xlrec + SizeOfHeapDelete,
-						len, change->data.tp.oldtuple);
+						datalen, change->data.tp.oldtuple);
 	}
 
 	change->data.tp.clear_toast_afterwards = true;
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
@@ -469,12 +469,15 @@ ReorderBufferGetTupleBuf(ReorderBuffer *rb, Size tuple_len)
 		rb->nr_cached_tuplebufs--;
 		tuple = slist_container(ReorderBufferTupleBuf, node,
 								slist_pop_head_node(&rb->cached_tuplebufs));
+		Assert(tuple->alloc_tuple_size == MaxHeapTupleSize);
 #ifdef USE_ASSERT_CHECKING
 		memset(&tuple->tuple, 0xa9, sizeof(HeapTupleData));
+		VALGRIND_MAKE_MEM_UNDEFINED(&tuple->tuple, sizeof(HeapTupleData));
 #endif
 		tuple->tuple.t_data = ReorderBufferTupleBufData(tuple);
 #ifdef USE_ASSERT_CHECKING
 		memset(tuple->tuple.t_data, 0xa8, tuple->alloc_tuple_size);
+		VALGRIND_MAKE_MEM_UNDEFINED(tuple->tuple.t_data, tuple->alloc_tuple_size);
 #endif
 	}
 	else
