[svn] commit: r1118 - in /trunk/src: bin/auth/auth_srv.cc bin/auth/auth_srv.h bin/auth/main.cc lib/auth/cpp/data_source.cc lib/auth/cpp/data_source.h

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Mar 5 00:02:08 UTC 2010


Author: jinmei
Date: Fri Mar  5 00:02:08 2010
New Revision: 1118

Log:
- made AuthSrv construction exception-safe
- fixed memory leak for Datasrc* stored in the MetaDataSrc vector.
  there are several possible ways to do this, but I chose to using
  boost::shared_ptr.  expect for portability issues this seems to be the
  cleanest solution, and, regarding portability, we already heavily rely on
  boost anyway, so we should revisit the whole design if/when we seriously
  consider binary portability.

Modified:
    trunk/src/bin/auth/auth_srv.cc
    trunk/src/bin/auth/auth_srv.h
    trunk/src/bin/auth/main.cc
    trunk/src/lib/auth/cpp/data_source.cc
    trunk/src/lib/auth/cpp/data_source.h

Modified: trunk/src/bin/auth/auth_srv.cc
==============================================================================
--- trunk/src/bin/auth/auth_srv.cc (original)
+++ trunk/src/bin/auth/auth_srv.cc Fri Mar  5 00:02:08 2010
@@ -21,6 +21,7 @@
 #include <netdb.h>
 #include <stdlib.h>
 
+#include <cassert>
 #include <iostream>
 
 #include <dns/buffer.h>
@@ -33,6 +34,9 @@
 #include <config/ccsession.h>
 
 #include <auth/query.h>
+#include <auth/data_source.h>
+#include <auth/data_source_static.h>
+#include <auth/data_source_sqlite3.h>
 
 #include <cc/data.h>
 
@@ -49,11 +53,27 @@
 using namespace isc::data;
 using namespace isc::config;
 
-AuthSrv::AuthSrv(int port) :
-    data_src(NULL), sock(-1)
-{
-    int s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
-    if (s < 0) {
+namespace {
+// This is a helper class to make construction of the AuthSrv class
+// exception safe.
+class AuthSocket {
+private:
+    // prohibit copy
+    AuthSocket(const AuthSocket& source);
+    AuthSocket& operator=(const AuthSocket& source);
+public:
+    AuthSocket(int port);
+    ~AuthSocket();
+    int getFD() const { return (fd_); }
+private:
+    int fd_;
+};
+
+AuthSocket::AuthSocket(int port) :
+    fd_(-1)
+{
+    fd_ = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    if (fd_ < 0) {
         throw FatalError("failed to open socket");
     }
 
@@ -67,34 +87,56 @@
     sin.sin_len = sa_len;
 #endif
 
-    if (bind(s, (struct sockaddr *)&sin, sa_len) < 0) {
-        close(s);
+    if (bind(fd_, (const struct sockaddr *)&sin, sa_len) < 0) {
+        close(fd_);
         throw FatalError("could not bind socket");
     }
-
-    sock = s;
-
-    // XXX: the following code is not exception-safe.  Will address in the
-    // next phase.
-
-    data_src = new(MetaDataSrc);
-
+}
+
+AuthSocket::~AuthSocket() {
+    assert(fd_ >= 0);
+    close(fd_);
+}
+}
+
+class AuthSrvImpl {
+private:
+    // prohibit copy
+    AuthSrvImpl(const AuthSrvImpl& source);
+    AuthSrvImpl& operator=(const AuthSrvImpl& source);
+public:
+    AuthSrvImpl(int port);
+    AuthSocket sock;
+    std::string _db_file;
+    isc::auth::MetaDataSrc data_sources;
+};
+
+AuthSrvImpl::AuthSrvImpl(int port) :
+    sock(port)
+{
     // add static data source
-    data_src->addDataSrc(new StaticDataSrc);
+    data_sources.addDataSrc(ConstDataSrcPtr(new StaticDataSrc));
 
     // add SQL data source
     Sqlite3DataSrc* sd = new Sqlite3DataSrc;
     sd->init();
-    data_src->addDataSrc(sd);
+    data_sources.addDataSrc(ConstDataSrcPtr(sd));
+}
+
+AuthSrv::AuthSrv(int port)
+{
+    impl_ = new AuthSrvImpl(port);
 }
 
 AuthSrv::~AuthSrv()
 {
-    if (sock >= 0) {
-        close(sock);
-    }
-
-    delete data_src;
+    delete impl_;
+}
+
+int
+AuthSrv::getSocket() const
+{
+    return (impl_->sock.getFD());
 }
 
 void
@@ -103,7 +145,7 @@
     struct sockaddr_storage ss;
     socklen_t sa_len = sizeof(ss);
     struct sockaddr* sa = static_cast<struct sockaddr*>((void*)&ss);
-    int s = sock;
+    const int s = impl_->sock.getFD();
     char recvbuf[4096];
     int cc;
 
@@ -134,7 +176,7 @@
         msg.setUDPSize(sizeof(recvbuf));
 
         Query query(msg, dnssec_ok);
-        data_src->doQuery(query);
+        impl_->data_sources.doQuery(query);
 
         OutputBuffer obuffer(remote_bufsize);
         MessageRenderer renderer(obuffer);
@@ -150,7 +192,7 @@
 AuthSrv::setDbFile(const std::string& db_file)
 {
     cout << "Change data source file, call our data source's function to now read " << db_file << endl;
-    _db_file = db_file;
+    impl_->_db_file = db_file;
 }
 
 ElementPtr

Modified: trunk/src/bin/auth/auth_srv.h
==============================================================================
--- trunk/src/bin/auth/auth_srv.h (original)
+++ trunk/src/bin/auth/auth_srv.h Fri Mar  5 00:02:08 2010
@@ -20,23 +20,30 @@
 #include <string>
 
 #include <cc/data.h>
-#include <auth/data_source_static.h>
-#include <auth/data_source_sqlite3.h>
+
+class AuthSrvImpl;
 
 class AuthSrv {
+    ///
+    /// \name Constructors, Assignment Operator and Destructor.
+    ///
+    /// Note: The copy constructor and the assignment operator are intentionally
+    /// defined as private.
+    //@{
+private:
+    AuthSrv(const AuthSrv& source);
+    AuthSrv& operator=(const AuthSrv& source);
 public:
     explicit AuthSrv(int port);
     ~AuthSrv();
-    int getSocket() { return (sock); }
+    //@}
+    int getSocket() const;
     void processMessage();
     void serve(std::string zone_name);
     void setDbFile(const std::string& db_file);
     isc::data::ElementPtr updateConfig(isc::data::ElementPtr config);
 private:
-    std::string _db_file;
-
-    isc::auth::MetaDataSrc* data_src;
-    int sock;
+    AuthSrvImpl* impl_;
 };
 
 #endif // __AUTH_SRV_H

Modified: trunk/src/bin/auth/main.cc
==============================================================================
--- trunk/src/bin/auth/main.cc (original)
+++ trunk/src/bin/auth/main.cc Fri Mar  5 00:02:08 2010
@@ -48,7 +48,7 @@
 /* need global var for config/command handlers.
  * todo: turn this around, and put handlers in the authserver
  * class itself? */
-AuthSrv auth = AuthSrv(DNSPORT);
+AuthSrv auth(DNSPORT);
 
 static void
 usage() {

Modified: trunk/src/lib/auth/cpp/data_source.cc
==============================================================================
--- trunk/src/lib/auth/cpp/data_source.cc (original)
+++ trunk/src/lib/auth/cpp/data_source.cc Fri Mar  5 00:02:08 2010
@@ -645,24 +645,25 @@
 }
 
 void
-MetaDataSrc::addDataSrc(DataSrc* ds)
-{
-    if (getClass() != RRClass::ANY() && ds->getClass() != getClass()) {
+MetaDataSrc::addDataSrc(ConstDataSrcPtr data_src)
+{
+    if (getClass() != RRClass::ANY() && data_src->getClass() != getClass()) {
         dns_throw(Unexpected, "class mismatch");
     }
 
-    data_sources.push_back(ds);
+    data_sources.push_back(data_src);
 }
 
 void
 MetaDataSrc::findClosestEnclosure(NameMatch& match) const
 {
-    BOOST_FOREACH (DataSrc* ds, data_sources) {
-        if (getClass() != RRClass::ANY() && ds->getClass() != getClass()) {
+    BOOST_FOREACH (ConstDataSrcPtr data_src, data_sources) {
+        if (getClass() != RRClass::ANY() &&
+            data_src->getClass() != getClass()) {
             continue;
         }
 
-        ds->findClosestEnclosure(match);
+        data_src->findClosestEnclosure(match);
     }
 }
 

Modified: trunk/src/lib/auth/cpp/data_source.h
==============================================================================
--- trunk/src/lib/auth/cpp/data_source.h (original)
+++ trunk/src/lib/auth/cpp/data_source.h Fri Mar  5 00:02:08 2010
@@ -19,6 +19,8 @@
 
 #include <vector>
 
+#include <boost/shared_ptr.hpp>
+
 #include <dns/name.h>
 #include <dns/rrclass.h>
 
@@ -35,6 +37,10 @@
 
 class NameMatch;
 class Query;
+
+class DataSrc;
+typedef boost::shared_ptr<DataSrc> DataSrcPtr;
+typedef boost::shared_ptr<const DataSrc> ConstDataSrcPtr;
 
 class AbstractDataSrc {
     ///
@@ -220,7 +226,7 @@
     virtual ~MetaDataSrc() {}
     //@}
 
-    void addDataSrc(DataSrc* ds);
+    void addDataSrc(ConstDataSrcPtr data_src);
     void findClosestEnclosure(NameMatch& match) const;
 
     // Actual queries for data should not be sent to a MetaDataSrc object,
@@ -273,7 +279,7 @@
     }
 
 private:
-    std::vector<DataSrc*> data_sources;
+    std::vector<ConstDataSrcPtr> data_sources;
 };
 
 class NameMatch {




More information about the bind10-changes mailing list