[rhmessaging-commits] rhmessaging commits: r4013 - store/trunk/cpp/lib/jrnl.

[rhmessaging-commits at lists.jboss.org]

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;