Here's a patch to fix the problem I reported earlier today.
With it, all cpp tests pass (assuming the incoming s/507/530/),
and those in bdbstore no longer trigger a valgrind-used-uninitialized
error.

Note that this patch is minimal, just to fix the bug.
I can easily imagine that someone might want to protect the members
of the new AccumulatedAck class, add accessors, etc., but that
change belongs in a separate delta.

One final note, regarding this change, from below:
-            AccumulatedAck ack;
-            ack.clear();
+            AccumulatedAck ack(0);
If you'd like to keep the use of ack.clear (perhaps for better
test coverage), let me know and I'll resubmit with this instead:

            AccumulatedAck ack(42);
            ack.clear();

Here's the patch:

2006-12-08  Jim Meyering  <[EMAIL PROTECTED]>

        Ensure that AccumulatedAck.range is not used uninitialized.
        * lib/broker/AccumulatedAck.h (AccumulatedAck): Make this a class,
        rather than a struct.
        (AccumulatedAck::AccumulatedAck): Add a constructor to require
        initialization of the "range" member.
        * lib/broker/BrokerChannel.cpp (Channel) [accumulatedAck]: Initialize.
        * tests/TxAckTest.cpp (TxAckTest) [acked]: Likewise.
        * tests/AccumulatedAckTest.cpp (testCovers): Initialize local.
        (testUpdateAndConsolidate): Likewise.

Index: lib/broker/AccumulatedAck.h
===================================================================
--- lib/broker/AccumulatedAck.h (revision 495629)
+++ lib/broker/AccumulatedAck.h (working copy)
@@ -31,7 +31,8 @@
          * Keeps an accumulated record of acked messages (by delivery
          * tag).
          */
-        struct AccumulatedAck{
+        class AccumulatedAck {
+       public:
             /**
              * If not zero, then everything up to this value has been
              * acked.
@@ -43,6 +44,7 @@
              */
             std::list<u_int64_t> individual;

+            AccumulatedAck(u_int64_t r) : range(r) {}
             void update(u_int64_t tag, bool multiple);
             void consolidate();
             void clear();
Index: lib/broker/BrokerChannel.cpp
===================================================================
--- lib/broker/BrokerChannel.cpp        (revision 495629)
+++ lib/broker/BrokerChannel.cpp        (working copy)
@@ -40,6 +40,7 @@
     prefetchCount(0),
     framesize(_framesize),
     tagGenerator("sgen"),
+    accumulatedAck(0),
     store(_store),
     messageBuilder(this, _store, _stagingThreshold),
     version(_version){
Index: tests/TxAckTest.cpp
===================================================================
--- tests/TxAckTest.cpp (revision 495629)
+++ tests/TxAckTest.cpp (working copy)
@@ -66,7 +66,7 @@

 public:

-    TxAckTest() : queue(new Queue("my_queue", false, &store, 0)), op(acked, 
deliveries, &xid)
+    TxAckTest() : acked(0), queue(new Queue("my_queue", false, &store, 0)), 
op(acked, deliveries, &xid)
     {
         for(int i = 0; i < 10; i++){
             Message::shared_ptr msg(new Message(0, "exchange", "routing_key", 
false, false));
Index: tests/AccumulatedAckTest.cpp
===================================================================
--- tests/AccumulatedAckTest.cpp        (revision 495629)
+++ tests/AccumulatedAckTest.cpp        (working copy)
@@ -36,8 +36,7 @@
     public:
         void testCovers()
         {
-            AccumulatedAck ack;
-            ack.range = 5;
+            AccumulatedAck ack(5);
             ack.individual.push_back(7);
             ack.individual.push_back(9);
             
@@ -56,8 +55,7 @@

         void testUpdateAndConsolidate()
         {
-            AccumulatedAck ack;
-            ack.clear();
+            AccumulatedAck ack(0);
             ack.update(1, false);
             ack.update(3, false);
             ack.update(10, false);

Reply via email to