Unitialised variable bug in ICMPv4PacketImpl.cpp

[hubert]

There is an uninitialised variable bug in ICMPv4PacketImpl.cpp. initPacket() reads _seq which is not set in the constructor. The solution is to change line 127 to

Code: Select all

ICMPv4PacketImpl::ICMPv4PacketImpl(int dataSize):
   ICMPPacketImpl(dataSize),_seq(0)
{
   initPacket();
}


(Notice the _seq(0) at the end)

I found this bug by running code under valgrind which complains about uninitialised variables being read. Might I suggest that valgrind is used as part of the standard build process for Poco. I have tried running it on some of the ter st suites and there are a number of memory leaks. This appears to be because of using raw pointers instead of smart pointers in a number of functions:. For instance, Poco::ThreadPool::createThread() returns a raw pointer instead of auto_ptr or shared_ptr.
HTTPServerTest::testKeepAliveTimeout() could use and auto_ptr to avoid a leak or allocate HTTPServerParams on the stack if it didn't have a protected destructor. The answer in a previous post that these classes should be used with Poco::AutoPtr seems not to be borne out in practice.

[alex]

There is an uninitialised variable bug in ICMPv4PacketImpl.cpp. initPacket() reads _seq which is not set in the constructor. The solution is to change line 127 to

Code: Select all

ICMPv4PacketImpl::ICMPv4PacketImpl(int dataSize):
   ICMPPacketImpl(dataSize),_seq(0)
{
   initPacket();
}


(Notice the _seq(0) at the end)

I have filed SF patch ( https://sourceforge.net/tracker2/?func= ... tid=725711 ). Although admittedly untidy, this is non-critical.

I found this bug by running code under valgrind which complains about uninitialised variables being read. Might I suggest that valgrind is used as part of the standard build process for Poco.

You certainly may. If you'll volunteer to run valgrind, separate false from true positives and report them through SF tracker with tested diff-patches for future releases, we'll appreciate it very much.