From 804800864a9f1d6c0fbd84b105e4171e822a23c8 Mon Sep 17 00:00:00 2001 From: Daniel Adolfsson Date: Sat, 4 Feb 2012 00:23:11 +0100 Subject: [PATCH] Fix minor memory leak and logging --- src/iface.cc | 32 ++++++++++++++++--- src/iface.h | 88 +++++++++++++++++++++++++++------------------------ src/logger.cc | 11 +++++-- src/logger.h | 3 +- src/ndppd.cc | 25 ++++++++++++--- 5 files changed, 104 insertions(+), 55 deletions(-) diff --git a/src/iface.cc b/src/iface.cc index 9a04f05..3996250 100644 --- a/src/iface.cc +++ b/src/iface.cc @@ -44,6 +44,8 @@ NDPPD_NS_BEGIN std::map > iface::_map; +bool iface::_map_dirty = false; + std::vector iface::_pollfds; iface::iface() : @@ -65,7 +67,7 @@ iface::~iface() close(_pfd); } - _map.erase(_name); + _map_dirty = true; } ptr iface::open_pfd(const std::string& name) @@ -451,6 +453,10 @@ ssize_t iface::read_advert(address& saddr, address& taddr) void iface::fixup_pollfds() { + if (_map_dirty) { + clean(); + } + _pollfds.resize(_map.size()* 2); int i = 0; @@ -487,8 +493,24 @@ void iface::add_session(const ptr& se) _sessions.push_back(se); } +void iface::clean() +{ + for (std::map >::iterator it = _map.begin(); + it != _map.end(); it++) { + if (!it->second) { + _map.erase(it); + } + } + + _map_dirty = false; +} + int iface::poll_all() { + if (_map_dirty) { + fixup_pollfds(); + } + if (_pollfds.size() == 0) { ::sleep(1); return 0; @@ -498,11 +520,13 @@ int iface::poll_all() int len; - if ((len = ::poll(&_pollfds[0], _pollfds.size(), 50)) < 0) + if ((len = ::poll(&_pollfds[0], _pollfds.size(), 50)) < 0) { return -1; + } - if (len == 0) + if (len == 0) { return 0; + } std::map >::iterator i_it = _map.begin(); @@ -518,7 +542,7 @@ int iface::poll_all() bool is_pfd = i++ % 2; - if (!(f_it->revents& POLLIN)) { + if (!(f_it->revents & POLLIN)) { continue; } diff --git a/src/iface.h b/src/iface.h index 80bc01e..be4f050 100644 --- a/src/iface.h +++ b/src/iface.h @@ -31,48 +31,6 @@ class session; class proxy; class iface { -private: - // Weak pointer so this object can reference itself. - weak_ptr _ptr; - - static std::map > _map; - - // An array of objects used with ::poll. - static std::vector _pollfds; - - // Updates the array above. - static void fixup_pollfds(); - - // The "generic" ICMPv6 socket for reading/writing NB_NEIGHBOR_ADVERT - // messages as well as writing NB_NEIGHBOR_SOLICIT messages. - int _ifd; - - // This is the PF_PACKET socket we use in order to read - // NB_NEIGHBOR_SOLICIT messages. - int _pfd; - - // Previous state of ALLMULTI for the interface. - int _prev_allmulti; - - // Name of this interface. - std::string _name; - - // An array of sessions that are monitoring this interface for - // ND_NEIGHBOR_ADVERT messages. - std::list > _sessions; - - weak_ptr _pr; - - // The link-layer address of this interface. - struct ether_addr hwaddr; - - // Turns on/off ALLMULTI for this interface - returns the previous state - // or -1 if there was an error. - int allmulti(int state); - - // Constructor. - iface(); - public: // Destructor. @@ -111,6 +69,52 @@ public: void pr(const ptr& pr); const ptr& pr() const; + +private: + static std::map > _map; + + static bool _map_dirty; + + // An array of objects used with ::poll. + static std::vector _pollfds; + + // Updates the array above. + static void fixup_pollfds(); + + static void clean(); + + // Weak pointer so this object can reference itself. + weak_ptr _ptr; + + // The "generic" ICMPv6 socket for reading/writing NB_NEIGHBOR_ADVERT + // messages as well as writing NB_NEIGHBOR_SOLICIT messages. + int _ifd; + + // This is the PF_PACKET socket we use in order to read + // NB_NEIGHBOR_SOLICIT messages. + int _pfd; + + // Previous state of ALLMULTI for the interface. + int _prev_allmulti; + + // Name of this interface. + std::string _name; + + // An array of sessions that are monitoring this interface for + // ND_NEIGHBOR_ADVERT messages. + std::list > _sessions; + + weak_ptr _pr; + + // The link-layer address of this interface. + struct ether_addr hwaddr; + + // Turns on/off ALLMULTI for this interface - returns the previous state + // or -1 if there was an error. + int allmulti(int state); + + // Constructor. + iface(); }; NDPPD_NS_END diff --git a/src/logger.cc b/src/logger.cc index 7ac4b58..01bdbc2 100644 --- a/src/logger.cc +++ b/src/logger.cc @@ -38,7 +38,7 @@ NDPPD_NS_BEGIN "debug" };*/ -int logger::_max_pri = LOG_WARNING; +int logger::_max_pri = LOG_NOTICE; bool logger::_syslog = false; @@ -100,6 +100,11 @@ logger logger::debug() return logger(LOG_DEBUG); } +logger logger::notice() +{ + return logger(LOG_NOTICE); +} + logger& logger::operator<<(const std::string& str) { _ss << str; @@ -140,12 +145,12 @@ void logger::flush() #ifndef DISABLE_SYSLOG if (_syslog) { - ::syslog(_pri, "%s", _ss.str().c_str()); + ::syslog(_pri, "(%s) %s", _pri_names[_pri].name, _ss.str().c_str()); return; } #endif - std::cout << _ss.str() << std::endl; + std::cout << "(" << _pri_names[_pri].name << ") " << _ss.str() << std::endl; _ss.str(""); } diff --git a/src/logger.h b/src/logger.h index 5e47f02..f97ea55 100644 --- a/src/logger.h +++ b/src/logger.h @@ -34,7 +34,7 @@ NDPPD_NS_BEGIN class logger { public: - logger(int pri = LOG_INFO); + logger(int pri = LOG_NOTICE); logger(const logger& l); @@ -69,6 +69,7 @@ public: static logger info(); static logger warning(); static logger debug(); + static logger notice(); private: int _pri; diff --git a/src/ndppd.cc b/src/ndppd.cc index ed09e71..628beea 100644 --- a/src/ndppd.cc +++ b/src/ndppd.cc @@ -118,9 +118,17 @@ bool configure(const std::string& path) } else if (ru_cf->find("auto")) { pr->add_rule(addr, true); } else { - if (addr.prefix() <= 120) { - logger::warning() << "Static rule prefix /" << addr.prefix() << " <= 120 - is this what you want?"; + if (!ru_cf->find("static")) { + logger::warning() + << "## I'm going for 'static' since you didn't specify any method. Please fix this" << logger::endl + << "## as it's not going to be supported in future versions of ndppd. (See 'man ndppd.conf')"; } + + if (addr.prefix() <= 120) { + logger::warning() + << "Low prefix length (" << addr.prefix() << " <= 120) when using 'static' method"; + } + pr->add_rule(addr, false); } } @@ -204,7 +212,7 @@ int main(int argc, char* argv[], char* env[]) pf.close(); } - logger::info().force_log() + logger::notice() << "ndppd (NDP Proxy Daemon) version " NDPPD_VERSION << logger::endl << "Using configuration file '" << config_path << "'"; @@ -221,7 +229,14 @@ int main(int argc, char* argv[], char* env[]) gettimeofday(&t1, 0); - while (running && (iface::poll_all() >= 0)) { + while (running) { + if (iface::poll_all() < 0) { + if (running) { + logger::error() << "iface::poll_all() failed"; + } + break; + } + int elapsed_time; gettimeofday(&t2, 0); @@ -235,7 +250,7 @@ int main(int argc, char* argv[], char* env[]) session::update_all(elapsed_time); } - logger::error() << "iface::poll_all() failed"; + logger::notice() << "Bye"; return 0; }