Author: kpvdr
Date: 2010-06-07 13:36:52 -0400 (Mon, 07 Jun 2010)
New Revision: 4013
Modified:
store/trunk/cpp/lib/jrnl/enq_rec.cpp
Log:
Added TODO comment for code improvement
Modified: store/trunk/cpp/lib/jrnl/enq_rec.cpp
===================================================================
--- store/trunk/cpp/lib/jrnl/enq_rec.cpp 2010-06-07 14:17:33 UTC (rev 4012)
+++ store/trunk/cpp/lib/jrnl/enq_rec.cpp 2010-06-07 17:36:52 UTC (rev 4013)
@@ -307,14 +307,29 @@
else if (hdr_data_dblks - rec_offs_dblks <= max_size_dblks)
{
// Remainder of xid & data fits within this page; tail split
- if (offs < _enq_hdr._xidsize)
+
+ /*
+ * TODO: This section needs revision. Since it is known that the end of the
page falls within the
+ * tail record, it is only necessary to write from the current offset to the
end of the page under
+ * all circumstances. The multiple if/else combinations may be eliminated, as
well as one memcpy()
+ * operation.
+ *
+ * Also note that Coverity has detected a possible memory overwrite in this
block. It occurs if
+ * both the following two if() stmsts (numbered) are false. With rd_cnt = 0,
this would result in
+ * the value of tail_rem > sizeof(tail_rec). Practically, this could only
happen if the start and
+ * end of a page both fall within the same tail record, in which case the
tail would have to be
+ * (much!) larger. However, the logic here does not account for this
possibility.
+ *
+ * If the optimization above is undertaken, this code would probably be
removed.
+ */
+ if (offs < _enq_hdr._xidsize) // 1
{
// some XID still outstanding, copy remainder of XID and data
const std::size_t rem = _enq_hdr._xidsize + _enq_hdr._dsize - offs;
std::memcpy((char*)_buff + offs, rptr, rem);
rd_cnt += rem;
}
- else if (offs < _enq_hdr._xidsize + _enq_hdr._dsize &&
!_enq_hdr.is_external())
+ else if (offs < _enq_hdr._xidsize + _enq_hdr._dsize &&
!_enq_hdr.is_external()) // 2
{
// some data still outstanding, copy remainder of data
const std::size_t data_offs = offs - _enq_hdr._xidsize;