qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Meyering <...@meyering.net>
Subject C++: ack.range used uninitialized via bdbstore tests
Date Fri, 12 Jan 2007 15:50:43 GMT
In testing the bdbstore code, I found that AccumulatedAck.range
is used uninitialized.  Here's part of valgrind's report:

==3863== Thread 7:
==3863== Conditional jump or move depends on uninitialised value(s)
==3863==    at 0x4B4A63F: qpid::broker::AccumulatedAck::update(unsigned long, bool) (AccumulatedAck.cpp:29)
==3863==    by 0x4B6DE21: qpid::broker::SessionHandlerImpl::BasicHandlerImpl::ack(unsigned
short, unsigned long, bool) (SessionHandlerImpl.cpp:427)
==3863==    by 0x4B6E8EF: qpid::broker::SessionHandlerImpl::received(qpid::framing::AMQFrame*)
==3863==    by 0x4CF5534: qpid::sys::LFSessionContext::read() (LFSessionContext.cpp:63)
==3863==    by 0x4CF44DD: qpid::sys::LFProcessor::run() (LFProcessor.cpp:125)
==3863==    by 0x4CF70A3: qpid::sys::Thread::runRunnable(apr_thread_t*, void*) (Thread.cpp:28)
==3863==    by 0x51BCF19: start_thread (in /usr/lib/debug/libpthread-2.3.6.so)
==3863==    by 0x5A2E5C1: clone (clone.S:112)

The code in question:

    25	void AccumulatedAck::update(u_int64_t tag, bool multiple){
    26	    if(multiple){
    27	        if(tag > range) range = tag;
    28	        //else don't care, it is already counted
    29	    }else if(tag > range){
    30	        individual.push_back(tag);
    31	    }
    32	}

Not surprisingly, the uses in qpidc's own tests do set the member first:

  ./tests/TxAckTest.cpp:        acked.range = 5;
  ./tests/AccumulatedAckTest.cpp:            ack.range = 5;

What do you think about making AccumulatedAck a class with the
sole constructor requiring an initializer for that member?
That should make it so this can't happen again.

View raw message