From: g0dil <g0dil@270642c3-0616-0410-b53a-bc976706d245>
Date: Tue, 13 Nov 2007 14:58:50 +0000 (+0000)
Subject: Socket/Protocols/Inet: Fix off-by-one error in INet6Address
X-Git-Url: http://g0dil.de/git?a=commitdiff_plain;h=44c966bc9d744d0926cffd5184fdb77a62564c16;p=senf.git

Socket/Protocols/Inet: Fix off-by-one error in INet6Address
PPI: Fix cloneSource unit-test packet construction
Scheduler: BUGFIX: uninitialized loop variable
Scheduler: BUGFIX: delay deletions in fdTable_ (can't change map while iterating over it)
Add global 'valgrind' target
Add 'valgrind.sup' suppressions file to suppress custom benign valgrind errors
Fix all unit-tests to run correcly under valgrind (mostly timing adjustments)


git-svn-id: https://svn.berlios.de/svnroot/repos/senf/trunk@511 270642c3-0616-0410-b53a-bc976706d245
---

diff --git a/PPI/CloneSource.test.cc b/PPI/CloneSource.test.cc
index 1b47375..14aeab5 100644
--- a/PPI/CloneSource.test.cc
+++ b/PPI/CloneSource.test.cc
@@ -40,7 +40,7 @@
 
 BOOST_AUTO_UNIT_TEST(cloneSource)
 {
-    char data[] = { 0xAB };
+    senf::PacketData::byte data[] = { 0xAB };
     senf::Packet p (senf::DataPacket::create(data));
 
     senf::ppi::module::CloneSource source (p);
diff --git a/PPI/IntervalTimer.test.cc b/PPI/IntervalTimer.test.cc
index f2e92e6..9aca3db 100644
--- a/PPI/IntervalTimer.test.cc
+++ b/PPI/IntervalTimer.test.cc
@@ -65,7 +65,7 @@ namespace {
     };
 
     bool is_close_clock(senf::ClockService::clock_type a, senf::ClockService::clock_type b, 
-                        unsigned long delta = 50000000ul)
+                        unsigned long delta = senf::ClockService::milliseconds(100))
     {
         return (a<b ? b-a : a-b ) < delta;
     }
diff --git a/SConstruct b/SConstruct
index 478cfdf..b501151 100644
--- a/SConstruct
+++ b/SConstruct
@@ -8,8 +8,8 @@ import SENFSCons
 
 # This hack is needed for SCons V 0.96.1 compatibility. In current SCons versions
 # we can just use 'env.AlwaysBuild(env.Alias(target), [], action)'
-def PhonyTarget(env, target, action):
-    env.AlwaysBuild(env.Command(target + '.phony', 'SConstruct', env.Action(action)))
+def PhonyTarget(env, target, action, sources=[]):
+    env.AlwaysBuild(env.Command(target + '.phony', [ 'SConstruct' ] + sources, env.Action(action)))
     env.Alias(target, target + '.phony')
 
 def updateRevision(target, source, env):
@@ -239,6 +239,10 @@ PhonyTarget(env, 'fixlinks', [
 
 PhonyTarget(env, 'prepare', [])
 
+PhonyTarget(env, 'valgrind', [
+    'find -name .test.bin | while read test; do echo; echo "Running $$test"; echo; valgrind --tool=memcheck --error-exitcode=99 --suppressions=valgrind.sup $$test $BOOSTTESTARGS; [ $$? -ne 99 ] || exit 1; done'
+    ], [ 'all_tests' ])
+
 env.Clean('all', env.Dir('linklint'))
 
 env.Clean('all','.prepare-stamp')
diff --git a/Scheduler/ClockService.test.cc b/Scheduler/ClockService.test.cc
index 1231e5d..2484d74 100644
--- a/Scheduler/ClockService.test.cc
+++ b/Scheduler/ClockService.test.cc
@@ -52,13 +52,13 @@ namespace detail {
 namespace {
 
     bool is_close_clock(senf::ClockService::clock_type a, senf::ClockService::clock_type b, 
-                        unsigned long delta = 50000000ul)
+                        unsigned long delta = senf::ClockService::milliseconds(100))
     {
         return (a<b ? b-a : a-b ) < delta;
     }
 
     bool is_close_pt(boost::posix_time::ptime a, boost::posix_time::ptime b,
-                     boost::posix_time::time_duration delta = boost::posix_time::milliseconds(50) )
+                     boost::posix_time::time_duration delta = boost::posix_time::milliseconds(100) )
     {
         return (a<b ? b-a : a-b ) < delta;
     }
@@ -105,7 +105,8 @@ BOOST_AUTO_UNIT_TEST(clockService)
     t2 = senf::ClockService::now();
     BOOST_CHECK_PREDICATE( is_close_clock,
                            (t1 + senf::ClockService::seconds(senf::ClockService::CheckInterval))
-                           (t2) );
+                           (t2)
+                           (senf::ClockService::milliseconds(500)) );
 
     t1 = t2;
 
diff --git a/Scheduler/Scheduler.cc b/Scheduler/Scheduler.cc
index c720c55..4170851 100644
--- a/Scheduler/Scheduler.cc
+++ b/Scheduler/Scheduler.cc
@@ -150,7 +150,7 @@ prefix_ void senf::Scheduler::do_remove(int fd, int eventMask)
     bool file (i->second.file);
     if (ev.events==0) {
         action = EPOLL_CTL_DEL;
-        fdTable_.erase(i);
+        fdErase_.push_back(fd);
     }
 
     if (! file && epoll_ctl(epollFd_, action, fd, &ev) < 0)
@@ -161,7 +161,7 @@ prefix_ void senf::Scheduler::do_remove(int fd, int eventMask)
 
 prefix_ void senf::Scheduler::registerSigHandlers()
 {
-    for (unsigned signal; signal < sigHandlers_.size(); ++signal)
+    for (unsigned signal (1); signal < sigHandlers_.size(); ++signal) {
         if (sigHandlers_[signal]) {
             struct ::sigaction sa;
             sa.sa_sigaction = & Scheduler::sigHandler;
@@ -172,6 +172,7 @@ prefix_ void senf::Scheduler::registerSigHandlers()
             if (::sigaction(signal, &sa, 0) < 0)
                 throw SystemException(errno);
         }
+    }
 }
 
 prefix_ void senf::Scheduler::sigHandler(int signal, ::siginfo_t * siginfo, void *)
@@ -214,6 +215,10 @@ prefix_ void senf::Scheduler::process()
             timerQueue_.pop();
         }
 
+        for (FdEraseList::iterator i (fdErase_.begin()); i != fdErase_.end(); ++i) 
+            fdTable_.erase(*i);
+        fdErase_.clear();
+
         int timeout (-1);
         if (files_ > 0)
             timeout = 0;
@@ -258,11 +263,8 @@ prefix_ void senf::Scheduler::process()
             timerMap_.erase(i);
         }
 
-        if (events <= 0)
-            continue;
-
         // Check the signal queue
-        if (ev.data.fd == sigpipe_[0]) {
+        if (events > 0 && ev.data.fd == sigpipe_[0]) {
             ::siginfo_t siginfo;
             if (::read(sigpipe_[0], &siginfo, sizeof(siginfo)) < int(sizeof(siginfo))) {
                 // We ignore truncated records which may only occur if the signal
@@ -276,25 +278,26 @@ prefix_ void senf::Scheduler::process()
         }
 
         for (FdTable::iterator i = fdTable_.begin(); i != fdTable_.end(); ++i) {
-            EventSpec spec (i->second);
-            unsigned extraFlags (0);
-            unsigned events (spec.file ? spec.epollMask() : ev.events);
+            EventSpec & spec (i->second);
 
-            if (! (spec.file || i->first == ev.data.fd))
+            if (! (spec.file || (events > 0 && i->first == ev.data.fd)))
                 continue;
                 
-            if (events & EPOLLHUP) extraFlags |= EV_HUP;
-            if (events & EPOLLERR) extraFlags |= EV_ERR;
+            unsigned extraFlags (0);
+            unsigned mask (spec.file ? spec.epollMask() : ev.events);
+
+            if (mask & EPOLLHUP) extraFlags |= EV_HUP;
+            if (mask & EPOLLERR) extraFlags |= EV_ERR;
 
-            if (events & EPOLLIN) {
+            if (mask & EPOLLIN) {
                 BOOST_ASSERT(spec.cb_read);
                 spec.cb_read(EventId(EV_READ | extraFlags));
             }
-            else if (events & EPOLLPRI) {
+            else if (mask & EPOLLPRI) {
                 BOOST_ASSERT(spec.cb_prio);
                 spec.cb_prio(EventId(EV_PRIO | extraFlags));
             }
-            else if (events & EPOLLOUT) {
+            else if (mask & EPOLLOUT) {
                 BOOST_ASSERT(spec.cb_write);
                 spec.cb_write(EventId(EV_WRITE | extraFlags));
             }
diff --git a/Scheduler/Scheduler.hh b/Scheduler/Scheduler.hh
index 4691e4a..94918af 100644
--- a/Scheduler/Scheduler.hh
+++ b/Scheduler/Scheduler.hh
@@ -351,6 +351,7 @@ namespace senf {
 
         typedef std::map<int,EventSpec> FdTable;
         typedef std::map<unsigned,TimerSpec> TimerMap; // sorted by id
+        typedef std::vector<unsigned> FdEraseList;
 
 #       ifndef DOXYGEN
 
@@ -371,6 +372,7 @@ namespace senf {
         typedef std::vector<SimpleCallback> SigHandlers;
 
         FdTable fdTable_;
+        FdEraseList fdErase_;
         unsigned files_;
 
         unsigned timerIdCounter_;
diff --git a/Scheduler/Scheduler.test.cc b/Scheduler/Scheduler.test.cc
index c26b640..2bc2a32 100644
--- a/Scheduler/Scheduler.test.cc
+++ b/Scheduler/Scheduler.test.cc
@@ -186,7 +186,7 @@ namespace {
 
     bool is_close(ClockService::clock_type a, ClockService::clock_type b)
     {
-        return (a<b ? b-a : a-b) < ClockService::milliseconds(15);
+        return (a<b ? b-a : a-b) < ClockService::milliseconds(100);
     }
     
     ClockService::clock_type sigtime (0);
@@ -240,14 +240,14 @@ BOOST_AUTO_UNIT_TEST(scheduler)
     BOOST_CHECK_EQUAL( buffer, "READ" );
 
     BOOST_CHECK_NO_THROW( Scheduler::instance().timeout(
-                              ClockService::now()+ClockService::milliseconds(100),&timeout) );
-    BOOST_CHECK_NO_THROW( Scheduler::instance().timeout(
                               ClockService::now()+ClockService::milliseconds(200),&timeout) );
+    BOOST_CHECK_NO_THROW( Scheduler::instance().timeout(
+                              ClockService::now()+ClockService::milliseconds(400),&timeout) );
     ClockService::clock_type t (ClockService::now());
     BOOST_CHECK_NO_THROW( Scheduler::instance().process() );
-    BOOST_CHECK_PREDICATE( is_close, (ClockService::now()) (t+ClockService::milliseconds(100)) );
-    BOOST_CHECK_NO_THROW( Scheduler::instance().process() );
     BOOST_CHECK_PREDICATE( is_close, (ClockService::now()) (t+ClockService::milliseconds(200)) );
+    BOOST_CHECK_NO_THROW( Scheduler::instance().process() );
+    BOOST_CHECK_PREDICATE( is_close, (ClockService::now()) (t+ClockService::milliseconds(400)) );
 
     HandleWrapper handle(sock,"TheTag");
     BOOST_CHECK_NO_THROW( Scheduler::instance().add(handle,
@@ -270,14 +270,14 @@ BOOST_AUTO_UNIT_TEST(scheduler)
     BOOST_CHECK_NO_THROW( Scheduler::instance().remove(handle) );
 
     unsigned tid (Scheduler::instance().timeout(
-                      ClockService::now()+ClockService::milliseconds(200),&timeout));
+                      ClockService::now()+ClockService::milliseconds(400),&timeout));
     BOOST_CHECK_NO_THROW( Scheduler::instance().registerSignal(SIGUSR1, &sigusr) );
     t = ClockService::now();
     ::kill(::getpid(), SIGUSR1);
     delay(100);
     BOOST_CHECK_NO_THROW( Scheduler::instance().process() ); 
-    BOOST_CHECK_PREDICATE( is_close, (ClockService::now()) (t+ClockService::milliseconds(100)) );
-    BOOST_CHECK_PREDICATE( is_close, (sigtime) (t+ClockService::milliseconds(100)) );
+    BOOST_CHECK_PREDICATE( is_close, (ClockService::now()) (t+ClockService::milliseconds(200)) );
+    BOOST_CHECK_PREDICATE( is_close, (sigtime) (t+ClockService::milliseconds(200)) );
     Scheduler::instance().cancelTimeout(tid);
     BOOST_CHECK_NO_THROW( Scheduler::instance().unregisterSignal(SIGUSR1) );
 
diff --git a/Socket/FileHandle.test.cc b/Socket/FileHandle.test.cc
index 014d072..2b9395b 100644
--- a/Socket/FileHandle.test.cc
+++ b/Socket/FileHandle.test.cc
@@ -97,6 +97,7 @@ BOOST_AUTO_UNIT_TEST(fileHandle)
             if (fcntl(fds[1],F_SETFL,flags|O_NONBLOCK) == -1)
                 BOOST_FAIL(strerror(errno));
             char buffer[1024];
+            ::memset(buffer, 0, sizeof(buffer));
             while (write(fds[1],buffer,1024) == 1024) ;
 
             FHandle fh2(fds[1]);
diff --git a/Socket/Protocols/INet/INet6Address.cc b/Socket/Protocols/INet/INet6Address.cc
index d330aba..93f5963 100644
--- a/Socket/Protocols/INet/INet6Address.cc
+++ b/Socket/Protocols/INet/INet6Address.cc
@@ -97,7 +97,7 @@ prefix_ std::ostream & senf::operator<<(std::ostream & os, INet6Address const &
     char buffer[5*8];
     std::copy(addr.begin(),addr.end(),&ina.s6_addr[0]);
     ::inet_ntop(AF_INET6,&ina,buffer,sizeof(buffer));
-    buffer[5*8] = 0;
+    buffer[sizeof(buffer)-1] = 0;
     os << buffer;
     return os;
 }
diff --git a/Socket/Protocols/INet/TCPSocketHandle.test.cc b/Socket/Protocols/INet/TCPSocketHandle.test.cc
index 3b4d049..8896f71 100644
--- a/Socket/Protocols/INet/TCPSocketHandle.test.cc
+++ b/Socket/Protocols/INet/TCPSocketHandle.test.cc
@@ -64,6 +64,7 @@ namespace {
             (*fn)();
             _exit(0);
         }
+        ::sleep(1);
     }
 
     void wait()
diff --git a/valgrind.sup b/valgrind.sup
new file mode 100644
index 0000000..4146e3e
--- /dev/null
+++ b/valgrind.sup
@@ -0,0 +1,103 @@
+{
+   gethostbyname2_r-suppression-1
+   Memcheck:Addr4
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:_dl_open
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:__libc_dlopen_mode
+   fun:__nss_lookup_function
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   fun:__nss_hosts_lookup
+   fun:gethostbyname2_r
+}
+{
+   gethostbyname2_r-suppression-2
+   Memcheck:Cond
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:_dl_open
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:__libc_dlopen_mode
+   fun:__nss_lookup_function
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   fun:__nss_hosts_lookup
+   fun:gethostbyname2_r
+}
+{
+   gethostbyname2_r-suppression-3
+   Memcheck:Cond
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:_dl_open
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:__libc_dlopen_mode
+   fun:__nss_lookup_function
+   fun:__nss_next
+   fun:gethostbyname2_r
+}
+{
+   gethostbyname2_r-suppression-4
+   Memcheck:Addr4
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:_dl_open
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:__libc_dlopen_mode
+   fun:__nss_lookup_function
+   fun:__nss_next
+   fun:gethostbyname2_r
+}
+{
+   gethostbyname2_r-suppression-5
+   Memcheck:Addr4
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:_dl_open
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:__libc_dlopen_mode
+   fun:__nss_lookup_function
+   fun:__nss_next
+   fun:gethostbyname2_r
+}
+{
+   gethostbyname2_r-suppression-4
+   Memcheck:Cond
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:_dl_open
+   obj:/lib/tls/i686/cmov/libc-2.3.6.so
+   obj:/lib/ld-2.3.6.so
+   fun:__libc_dlopen_mode
+   fun:__nss_lookup_function
+   fun:__nss_next
+   fun:gethostbyname2_r
+}
+
+{
+   inet_ntop-suppression-1
+   Memcheck:Cond
+   fun:inet_ntop
+}