database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-14 18:42:34 +03:00
Code Activity

Further improvements to c8f621c43.

Browse Source 
Coverity and inspection for the issue addressed in fd45d16f 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 c8f621c43.
This commit is contained in:
Andres Freund
2016-03-07 14:24:58 -08:00
parent 89f8372cb3
commit e3e84fd358
2 changed files with 18 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
src/backend/replication/logical/decode.c
View File

@ -625,7 +625,8 @@ DecodeInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
if (xlrec->flags & XLOG_HEAP_CONTAINS_NEW_TUPLE) 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)); Assert(r->xl_len > (SizeOfHeapInsert + SizeOfHeapHeader));
@ -633,7 +634,7 @@ DecodeInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
ReorderBufferGetTupleBuf(ctx->reorder, tuplelen); ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
DecodeXLogTuple((char *) xlrec + SizeOfHeapInsert, DecodeXLogTuple((char *) xlrec + SizeOfHeapInsert,
tuplelen, change->data.tp.newtuple); datalen, change->data.tp.newtuple);
} }
change->data.tp.clear_toast_afterwards = true; change->data.tp.clear_toast_afterwards = true;
@ -670,6 +671,7 @@ DecodeUpdate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
if (xlrec->flags & XLOG_HEAP_CONTAINS_NEW_TUPLE) if (xlrec->flags & XLOG_HEAP_CONTAINS_NEW_TUPLE)
{ {
Size datalen;
Size tuplelen; Size tuplelen;
xl_heap_header_len xlhdr; xl_heap_header_len xlhdr;
@ -678,12 +680,13 @@ DecodeUpdate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
memcpy(&xlhdr, data, sizeof(xlhdr)); memcpy(&xlhdr, data, sizeof(xlhdr));
data += offsetof(xl_heap_header_len, header); 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 = change->data.tp.newtuple =
ReorderBufferGetTupleBuf(ctx->reorder, tuplelen); 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 */ /* skip over the rest of the tuple header */
data += SizeOfHeapHeader; data += SizeOfHeapHeader;
/* skip over the tuple data */ /* skip over the tuple data */
@ -692,18 +695,20 @@ DecodeUpdate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
if (xlrec->flags & XLOG_HEAP_CONTAINS_OLD) if (xlrec->flags & XLOG_HEAP_CONTAINS_OLD)
{ {
Size datalen;
Size tuplelen; Size tuplelen;
xl_heap_header_len xlhdr; xl_heap_header_len xlhdr;
memcpy(&xlhdr, data, sizeof(xlhdr)); memcpy(&xlhdr, data, sizeof(xlhdr));
data += offsetof(xl_heap_header_len, header); 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 = change->data.tp.oldtuple =
ReorderBufferGetTupleBuf(ctx->reorder, tuplelen); ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
DecodeXLogTuple(data, tuplelen, change->data.tp.oldtuple); DecodeXLogTuple(data, datalen, change->data.tp.oldtuple);
#ifdef NOT_USED #ifdef NOT_USED
data += SizeOfHeapHeader; data += SizeOfHeapHeader;
data += xlhdr.t_len; data += xlhdr.t_len;
@ -741,15 +746,16 @@ DecodeDelete(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
/* old primary key stored */ /* old primary key stored */
if (xlrec->flags & XLOG_HEAP_CONTAINS_OLD) 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)); Assert(r->xl_len > (SizeOfHeapDelete + SizeOfHeapHeader));
change->data.tp.oldtuple = change->data.tp.oldtuple =
ReorderBufferGetTupleBuf(ctx->reorder, len); ReorderBufferGetTupleBuf(ctx->reorder, tuplelen);
DecodeXLogTuple((char *) xlrec + SizeOfHeapDelete, DecodeXLogTuple((char *) xlrec + SizeOfHeapDelete,
len, change->data.tp.oldtuple); datalen, change->data.tp.oldtuple);
} }
change->data.tp.clear_toast_afterwards = true; change->data.tp.clear_toast_afterwards = true;

3
src/backend/replication/logical/reorderbuffer.c
View File

@ -469,12 +469,15 @@ ReorderBufferGetTupleBuf(ReorderBuffer *rb, Size tuple_len)
rb->nr_cached_tuplebufs--; rb->nr_cached_tuplebufs--;
tuple = slist_container(ReorderBufferTupleBuf, node, tuple = slist_container(ReorderBufferTupleBuf, node,
slist_pop_head_node(&rb->cached_tuplebufs)); slist_pop_head_node(&rb->cached_tuplebufs));
Assert(tuple->alloc_tuple_size == MaxHeapTupleSize);
#ifdef USE_ASSERT_CHECKING #ifdef USE_ASSERT_CHECKING
memset(&tuple->tuple, 0xa9, sizeof(HeapTupleData)); memset(&tuple->tuple, 0xa9, sizeof(HeapTupleData));
VALGRIND_MAKE_MEM_UNDEFINED(&tuple->tuple, sizeof(HeapTupleData));
#endif #endif
tuple->tuple.t_data = ReorderBufferTupleBufData(tuple); tuple->tuple.t_data = ReorderBufferTupleBufData(tuple);
#ifdef USE_ASSERT_CHECKING #ifdef USE_ASSERT_CHECKING
memset(tuple->tuple.t_data, 0xa8, tuple->alloc_tuple_size); memset(tuple->tuple.t_data, 0xa8, tuple->alloc_tuple_size);
VALGRIND_MAKE_MEM_UNDEFINED(tuple->tuple.t_data, tuple->alloc_tuple_size);
#endif #endif
} }
else else