From 385c5936bd4bfab93068880d09e479a13c6ca6bd Mon Sep 17 00:00:00 2001
From: g0dil <g0dil@wiback.org>
Date: Tue, 13 Nov 2007 14:58:50 +0000
Subject: [PATCH] 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)
---
PPI/CloneSource.test.cc | 2 +-
PPI/IntervalTimer.test.cc | 2 +-
SConstruct | 8 +-
Scheduler/ClockService.test.cc | 7 +-
Scheduler/Scheduler.cc | 33 +++---
Scheduler/Scheduler.hh | 2 +
Scheduler/Scheduler.test.cc | 16 +--
Socket/FileHandle.test.cc | 1 +
Socket/Protocols/INet/INet6Address.cc | 2 +-
Socket/Protocols/INet/TCPSocketHandle.test.cc | 1 +
valgrind.sup | 103 ++++++++++++++++++
11 files changed, 146 insertions(+), 31 deletions(-)
create mode 100644 valgrind.sup
diff --git a/PPI/CloneSource.test.cc b/PPI/CloneSource.test.cc
index 1b47375de..14aeab5bf 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 f2e92e6a9..9aca3db54 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 478cfdf97..b501151c8 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 1231e5d8f..2484d74a7 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 c720c5509..41708511e 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 4691e4aa4..94918af13 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 c26b64072..2bc2a323a 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);
@@ -239,15 +239,15 @@ BOOST_AUTO_UNIT_TEST(scheduler)
buffer[size]=0;
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 014d07233..2b9395b9e 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 d330abad8..93f59638e 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 3b4d0494f..8896f716d 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 000000000..4146e3ee4
--- /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
+}
--
GitLab