Merge branch 'master' into trac2699_2

ChangeLog

@@ -1,3 +1,20 @@
+573.	[bug]		stephen
+	Fixed problem whereby the DHCP server crashed if it ran out of
+	addresses.  Such a condition now causes a packet to be returned
+	to the client refusing the allocation of an address.
+	(Trac #2681, git 87ce14cdb121b37afb5b1931af51bed7f6323dd6)
+
+572.	[bug]		marcin
+	perfdhcp: Fixed bug where the command line switches used to
+	run the perfdhcp where printed as ASCII codes.
+	(Trac #2700, git b8d6b949eb7f4705e32fbdfd7694ca2e6a6a5cdc)
+
+571.	[build]		jinmei
+	The ./configure script can now handle output from python-config
+	--ldflags that contains a space after -L switches.  This fixes
+	failure reported on some Solaris environments.
+	(Trac #2661, git e6f86f2f5eec8e6003c13d36804a767a840d96d6)
+
 570.	[bug]		tmark, marcin, tomek
 	b10-dhcp4: Address renewal now works properly for DHCPv4 clients
 	that do not send client ID.

configure.ac

@@ -313,8 +313,16 @@ AC_SUBST(COMMON_PYTHON_PATH)
 if test -x ${PYTHON}-config; then
 	PYTHON_INCLUDES=`${PYTHON}-config --includes`
 
-	for flag in `${PYTHON}-config --ldflags`; do
-		# add any '-L..." flags to PYTHON_LDFLAGS
+	# Add any '-L..." flags to PYTHON_LDFLAGS.  We first make a copy of
+	# python-config --ldflags, removing any spaces and tabs
+	# between "-L" and its argument (some instances of python-config
+	# insert a space, which would confuse the code below).
+	# Notes: if -L isn't contained at all we can simply skip this process,
+	# so we only go through the flag if it's contained; also, protecting
+	# the output with [] seems necessary for environment to avoid getting
+	# an empty output accidentally.
+	python_config_ldflags=[`${PYTHON}-config --ldflags | sed -ne 's/\([ \t]*-L\)[ ]*\([^ \t]*[ \t]*\)/\1\2/pg'`]
+	for flag in $python_config_ldflags; do
 		flag=`echo $flag | sed -ne 's/^\(\-L.*\)$/\1/p'`
 		if test "X${flag}" != X; then
 			PYTHON_LDFLAGS="$PYTHON_LDFLAGS ${flag}"

src/bin/dhcp4/dhcp4_messages.mes

@@ -1,4 +1,4 @@
-# Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2012-2013  Internet Systems Consortium, Inc. ("ISC")
 #
 # Permission to use, copy, modify, and/or distribute this software for any
 # purpose with or without fee is hereby granted, provided that the above
@@ -94,6 +94,11 @@ server is about to open sockets on the specified port.
 The IPv4 DHCP server has received a packet that it is unable to
 interpret. The reason why the packet is invalid is included in the message.
 
+% DHCP4_PACKET_PROCESS_FAIL failed to process packet received from %1: %2
+This is a general catch-all message indicating that the processing of a
+received packet failed.  The reason is given in the message.  The server
+will not send a response but will instead ignore the packet.
+
 % DHCP4_PACKET_RECEIVED %1 (type %2) packet received on interface %3
 A debug message noting that the server has received the specified type of
 packet on the specified interface.  Note that a packet marked as UNKNOWN

src/bin/dhcp4/dhcp4_srv.cc

@@ -149,32 +149,53 @@ Dhcpv4Srv::run() {
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_QUERY_DATA)
                       .arg(query->toText());
 
-            switch (query->getType()) {
-            case DHCPDISCOVER:
-                rsp = processDiscover(query);
-                break;
-
-            case DHCPREQUEST:
-                rsp = processRequest(query);
-                break;
-
-            case DHCPRELEASE:
-                processRelease(query);
-                break;
-
-            case DHCPDECLINE:
-                processDecline(query);
-                break;
-
-            case DHCPINFORM:
-                processInform(query);
-                break;
-
-            default:
-                // Only action is to output a message if debug is enabled,
-                // and that will be covered by the debug statement before
-                // the "switch" statement.
-                ;
+            try {
+                switch (query->getType()) {
+                case DHCPDISCOVER:
+                    rsp = processDiscover(query);
+                    break;
+
+                case DHCPREQUEST:
+                    rsp = processRequest(query);
+                    break;
+
+                case DHCPRELEASE:
+                    processRelease(query);
+                    break;
+
+                case DHCPDECLINE:
+                    processDecline(query);
+                    break;
+
+                case DHCPINFORM:
+                    processInform(query);
+                    break;
+
+                default:
+                    // Only action is to output a message if debug is enabled,
+                    // and that is covered by the debug statement before the
+                    // "switch" statement.
+                    ;
+                }
+            } catch (const isc::Exception& e) {
+
+                // Catch-all exception (at least for ones based on the isc
+                // Exception class, which covers more or less all that
+                // are explicitly raised in the BIND 10 code).  Just log
+                // the problem and ignore the packet. (The problem is logged
+                // as a debug message because debug is disabled by default -
+                // it prevents a DDOS attack based on the sending of problem
+                // packets.)
+                if (dhcp4_logger.isDebugEnabled(DBG_DHCP4_BASIC)) {
+                    std::string source = "unknown";
+                    HWAddrPtr hwptr = query->getHWAddr();
+                    if (hwptr) {
+                        source = hwptr->toText();
+                    }
+                    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC,
+                              DHCP4_PACKET_PROCESS_FAIL)
+                              .arg(source).arg(e.what());
+                }
             }
 
             if (rsp) {

src/bin/dhcp6/dhcp6_messages.mes

@@ -1,4 +1,4 @@
-# Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2012-2013  Internet Systems Consortium, Inc. ("ISC")
 #
 # Permission to use, copy, modify, and/or distribute this software for any
 # purpose with or without fee is hereby granted, provided that the above
@@ -93,6 +93,11 @@ This message indicates that the server failed to grant (in response to
 received REQUEST) a lease for a given client. There may be many reasons for
 such failure. Each specific failure is logged in a separate log entry.
 
+% DHCP6_PACKET_PROCESS_FAIL processing of %1 message received from %2 failed: %3
+This is a general catch-all message indicating that the processing of the
+specified packet type from the indicated address failed.  The reason is given in the
+message.  The server will not send a response but will instead ignore the packet.
+
 % DHCP6_RELEASE address %1 belonging to client duid=%2, iaid=%3 was released properly.
 This debug message indicates that an address was released properly. It
 is a normal operation during client shutdown.

src/bin/dhcp6/dhcp6_srv.cc

@@ -182,8 +182,8 @@ bool Dhcpv6Srv::run() {
                     break;
 
                 case DHCPV6_RELEASE:
-                rsp = processRelease(query);
-                break;
+                    rsp = processRelease(query);
+                    break;
 
                 case DHCPV6_DECLINE:
                     rsp = processDecline(query);
@@ -199,11 +199,26 @@ bool Dhcpv6Srv::run() {
                     // the "switch" statement.
                     ;
                 }
+
             } catch (const RFCViolation& e) {
                 LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_REQUIRED_OPTIONS_CHECK_FAIL)
                     .arg(query->getName())
                     .arg(query->getRemoteAddr())
                     .arg(e.what());
+
+            } catch (const isc::Exception& e) {
+
+                // Catch-all exception (at least for ones based on the isc
+                // Exception class, which covers more or less all that
+                // are explicitly raised in the BIND 10 code).  Just log
+                // the problem and ignore the packet. (The problem is logged
+                // as a debug message because debug is disabled by default -
+                // it prevents a DDOS attack based on the sending of problem
+                // packets.)
+                LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_PACKET_PROCESS_FAIL)
+                    .arg(query->getName())
+                    .arg(query->getRemoteAddr())
+                    .arg(e.what());
             }
 
             if (rsp) {
@@ -685,11 +700,10 @@ Dhcpv6Srv::assignIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
         // cause of that failure. The only thing left is to insert
         // status code to pass the sad news to the client.
 
-        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, fake_allocation?
-                  DHCP6_LEASE_ADVERT_FAIL:DHCP6_LEASE_ALLOC_FAIL)
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, fake_allocation ?
+                  DHCP6_LEASE_ADVERT_FAIL : DHCP6_LEASE_ALLOC_FAIL)
             .arg(duid?duid->toText():"(no-duid)")
-            .arg(ia->getIAID())
-            .arg(subnet->toText());
+            .arg(ia->getIAID());
 
         ia_rsp->addOption(createStatusCode(STATUS_NoAddrsAvail,
                           "Sorry, no address could be allocated."));

src/lib/asiodns/tcp_server.cc

@@ -184,7 +184,6 @@ TCPServer::operator()(asio::error_code ec, size_t length) {
         // provides the appropriate operator() but is otherwise functionless.
         iosock_.reset(new TCPSocket<DummyIOCallback>(*socket_));
         io_message_.reset(new IOMessage(data_.get(), length, *iosock_, *peer_));
-        bytes_ = length;
 
         // Perform any necessary operations prior to processing the incoming
         // packet (e.g., checking for queued configuration messages).

src/lib/asiodns/tcp_server.h

@@ -122,7 +122,6 @@ private:
 
     // State information that is entirely internal to a given instance
     // of the coroutine can be declared here.
-    size_t bytes_;
     bool done_;
 
     // Callback functions provided by the caller

src/lib/asiodns/udp_server.cc

@@ -61,7 +61,7 @@ struct UDPServer::Data {
      */
     Data(io_service& io_service, const ip::address& addr, const uint16_t port,
         SimpleCallback* checkin, DNSLookup* lookup, DNSAnswer* answer) :
-        io_(io_service), done_(false),
+        io_(io_service), bytes_(0), done_(false),
         checkin_callback_(checkin),lookup_callback_(lookup),
         answer_callback_(answer)
     {
@@ -77,7 +77,7 @@ struct UDPServer::Data {
     }
     Data(io_service& io_service, int fd, int af, SimpleCallback* checkin,
          DNSLookup* lookup, DNSAnswer* answer) :
-         io_(io_service), done_(false),
+         io_(io_service), bytes_(0), done_(false),
          checkin_callback_(checkin),lookup_callback_(lookup),
          answer_callback_(answer)
     {
@@ -104,7 +104,7 @@ struct UDPServer::Data {
      * We also allocate data for receiving the packet here.
      */
     Data(const Data& other) :
-        io_(other.io_), socket_(other.socket_), done_(false),
+        io_(other.io_), socket_(other.socket_), bytes_(0), done_(false),
         checkin_callback_(other.checkin_callback_),
         lookup_callback_(other.lookup_callback_),
         answer_callback_(other.answer_callback_)
@@ -168,7 +168,6 @@ struct UDPServer::Data {
     size_t bytes_;
     bool done_;
 
-
     // Callback functions provided by the caller
     const SimpleCallback* checkin_callback_;
     const DNSLookup* lookup_callback_;

src/lib/cc/data.cc

@@ -261,7 +261,7 @@ skipChars(std::istream& in, const char* chars, int& line, int& pos) {
         } else {
             ++pos;
         }
-        in.get();
+        in.ignore();
         c = in.peek();
     }
 }
@@ -291,7 +291,7 @@ skipTo(std::istream& in, const std::string& file, int& line,
                     pos = 1;
                     ++line;
                 }
-                in.get();
+                in.ignore();
                 ++pos;
             }
             in.putback(c);
@@ -352,7 +352,7 @@ strFromStringstream(std::istream& in, const std::string& file,
                 throwJSONError("Bad escape", file, line, pos);
             }
             // drop the escaped char
-            in.get();
+            in.ignore();
             ++pos;
         }
         ss.put(c);
@@ -490,14 +490,14 @@ fromStringstreamMap(std::istream& in, const std::string& file, int& line,
         throwJSONError(std::string("Unterminated map, <string> or } expected"), file, line, pos);
     } else if (c == '}') {
         // empty map, skip closing curly
-        c = in.get();
+        in.ignore();
     } else {
         while (c != EOF && c != '}') {
             std::string key = strFromStringstream(in, file, line, pos);
 
             skipTo(in, file, line, pos, ":", WHITESPACE);
             // skip the :
-            in.get();
+            in.ignore();
             pos++;
 
             ConstElementPtr value = Element::fromJSON(in, file, line, pos);

src/lib/config/tests/ccsession_unittests.cc

@@ -343,6 +343,7 @@ TEST_F(CCSessionTest, checkCommand) {
     session.addMessage(el("{ \"command\": \"bad_command\" }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(0, session.getMsgQueue()->size());
+    EXPECT_EQ(0, result);
 
     session.addMessage(el("{ \"command\": [ \"bad_command\" ] }"),
                        "Spec29", "*");
@@ -627,6 +628,7 @@ TEST_F(CCSessionTest, ignoreRemoteConfigCommands) {
     EXPECT_EQ(1, session.getMsgQueue()->size());
     result = mccs.checkCommand();
     EXPECT_EQ(0, session.getMsgQueue()->size());
+    EXPECT_EQ(0, result);
 }
 
 TEST_F(CCSessionTest, initializationFail) {

src/lib/datasrc/memory/zone_finder.cc

@@ -573,6 +573,7 @@ FindNodeResult findNode(const ZoneData& zone_data,
 /// For (successful) type ANY query, found_node points to the
 /// corresponding zone node, which is recorded within this specialized
 /// context.
+// cppcheck-suppress noConstructor
 class InMemoryZoneFinder::Context : public ZoneFinder::Context {
 public:
     Context(InMemoryZoneFinder& finder, ZoneFinder::FindOptions options,

src/lib/datasrc/memory_datasrc.cc

@@ -782,6 +782,7 @@ struct RBNodeResultContext {
 };
 }
 
+// cppcheck-suppress noConstructor
 class InMemoryZoneFinder::Context : public ZoneFinder::Context {
 public:
     /// \brief Constructor.

src/lib/datasrc/sqlite3_accessor.cc

@@ -682,6 +682,8 @@ convertToPlainChar(const unsigned char* ucp, sqlite3 *db) {
 }
 
 }
+
+// cppcheck-suppress noConstructor
 class SQLite3Accessor::Context : public DatabaseAccessor::IteratorContext {
 public:
     // Construct an iterator for all records. When constructed this
@@ -887,6 +889,7 @@ SQLite3Accessor::getAllRecords(int id) const {
 /// This iterator is used to search through the differences table for the
 /// resouce records making up an IXFR between two versions of a zone.
 
+// cppcheck-suppress noConstructor
 class SQLite3Accessor::DiffContext : public DatabaseAccessor::IteratorContext {
 public:
 

src/lib/dhcpsrv/alloc_engine.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <dhcpsrv/alloc_engine.h>
+#include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 
 #include <cstring>
@@ -169,95 +170,104 @@ AllocEngine::allocateAddress6(const Subnet6Ptr& subnet,
                               const IOAddress& hint,
                               bool fake_allocation /* = false */ ) {
 
-    // That check is not necessary. We create allocator in AllocEngine
-    // constructor
-    if (!allocator_) {
-        isc_throw(InvalidOperation, "No allocator selected");
-    }
-
-    // check if there's existing lease for that subnet/duid/iaid combination.
-    Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(*duid, iaid, subnet->getID());
-    if (existing) {
-        // we have a lease already. This is a returning client, probably after
-        // his reboot.
-        return (existing);
-    }
+    try {
+        // That check is not necessary. We create allocator in AllocEngine
+        // constructor
+        if (!allocator_) {
+            isc_throw(InvalidOperation, "No allocator selected");
+        }
 
-    // check if the hint is in pool and is available
-    if (subnet->inPool(hint)) {
-        existing = LeaseMgrFactory::instance().getLease6(hint);
-        if (!existing) {
-            /// @todo: check if the hint is reserved once we have host support
-            /// implemented
+        // check if there's existing lease for that subnet/duid/iaid combination.
+        Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(*duid, iaid, subnet->getID());
+        if (existing) {
+            // we have a lease already. This is a returning client, probably after
+            // his reboot.
+            return (existing);
+        }
 
-            // the hint is valid and not currently used, let's create a lease for it
-            Lease6Ptr lease = createLease6(subnet, duid, iaid, hint, fake_allocation);
+        // check if the hint is in pool and is available
+        if (subnet->inPool(hint)) {
+            existing = LeaseMgrFactory::instance().getLease6(hint);
+            if (!existing) {
+                /// @todo: check if the hint is reserved once we have host support
+                /// implemented
+
+                // the hint is valid and not currently used, let's create a lease for it
+                Lease6Ptr lease = createLease6(subnet, duid, iaid, hint, fake_allocation);
+
+                // It can happen that the lease allocation failed (we could have lost
+                // the race condition. That means that the hint is lo longer usable and
+                // we need to continue the regular allocation path.
+                if (lease) {
+                    return (lease);
+                }
+            } else {
+                if (existing->expired()) {
+                    return (reuseExpiredLease(existing, subnet, duid, iaid,
+                                              fake_allocation));
+                }
 
-            // It can happen that the lease allocation failed (we could have lost
-            // the race condition. That means that the hint is lo longer usable and
-            // we need to continue the regular allocation path.
-            if (lease) {
-                return (lease);
             }
-        } else {
-            if (existing->expired()) {
-                return (reuseExpiredLease(existing, subnet, duid, iaid,
-                                          fake_allocation));
-            }
-
         }
-    }
 
-    // Hint is in the pool but is not available. Search the pool until first of
-    // the following occurs:
-    // - we find a free address
-    // - we find an address for which the lease has expired
-    // - we exhaust number of tries
-    //
-    // @todo: Current code does not handle pool exhaustion well. It will be
-    // improved. Current problems:
-    // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
-    // 10 addresses), we will iterate over it 100 times before giving up
-    // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
-    // 3. the whole concept of infinite attempts is just asking for infinite loop
-    // We may consider some form or reference counting (this pool has X addresses
-    // left), but this has one major problem. We exactly control allocation
-    // moment, but we currently do not control expiration time at all
-
-    unsigned int i = attempts_;
-    do {
-        IOAddress candidate = allocator_->pickAddress(subnet, duid, hint);
-
-        /// @todo: check if the address is reserved once we have host support
-        /// implemented
-
-        Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(candidate);
-        if (!existing) {
-            // there's no existing lease for selected candidate, so it is
-            // free. Let's allocate it.
-            Lease6Ptr lease = createLease6(subnet, duid, iaid, candidate,
-                                          fake_allocation);
-            if (lease) {
-                return (lease);
-            }
+        // Hint is in the pool but is not available. Search the pool until first of
+        // the following occurs:
+        // - we find a free address
+        // - we find an address for which the lease has expired
+        // - we exhaust number of tries
+        //
+        // @todo: Current code does not handle pool exhaustion well. It will be
+        // improved. Current problems:
+        // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
+        // 10 addresses), we will iterate over it 100 times before giving up
+        // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
+        // 3. the whole concept of infinite attempts is just asking for infinite loop
+        // We may consider some form or reference counting (this pool has X addresses
+        // left), but this has one major problem. We exactly control allocation
+        // moment, but we currently do not control expiration time at all
+
+        unsigned int i = attempts_;
+        do {
+            IOAddress candidate = allocator_->pickAddress(subnet, duid, hint);
+
+            /// @todo: check if the address is reserved once we have host support
+            /// implemented
 
-            // Although the address was free just microseconds ago, it may have
-            // been taken just now. If the lease insertion fails, we continue
-            // allocation attempts.
-        } else {
-            if (existing->expired()) {
-                return (reuseExpiredLease(existing, subnet, duid, iaid,
-                                          fake_allocation));
+            Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(candidate);
+            if (!existing) {
+                // there's no existing lease for selected candidate, so it is
+                // free. Let's allocate it.
+                Lease6Ptr lease = createLease6(subnet, duid, iaid, candidate,
+                                              fake_allocation);
+                if (lease) {
+                    return (lease);
+                }
+
+                // Although the address was free just microseconds ago, it may have
+                // been taken just now. If the lease insertion fails, we continue
+                // allocation attempts.
+            } else {
+                if (existing->expired()) {
+                    return (reuseExpiredLease(existing, subnet, duid, iaid,
+                                              fake_allocation));
+                }
             }
-        }
 
-        // continue trying allocation until we run out of attempts
-        // (or attempts are set to 0, which means infinite)
-        --i;
-    } while ( i || !attempts_);
+            // Continue trying allocation until we run out of attempts
+            // (or attempts are set to 0, which means infinite)
+            --i;
+        } while ((i > 0) || !attempts_);
 
-    isc_throw(AllocFailed, "Failed to allocate address after " << attempts_
-              << " tries");
+        // Unable to allocate an address, return an empty lease.
+        LOG_WARN(dhcpsrv_logger, DHCPSRV_ADDRESS6_ALLOC_FAIL).arg(attempts_);
+
+    } catch (const isc::Exception& e) {
+
+        // Some other error, return an empty lease.
+        LOG_ERROR(dhcpsrv_logger, DHCPSRV_ADDRESS6_ALLOC_ERROR).arg(e.what());
+    }
+
+    return (Lease6Ptr());
 }
 
 Lease4Ptr
@@ -267,115 +277,123 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
                               const IOAddress& hint,
                               bool fake_allocation /* = false */ ) {
 
-    // Allocator is always created in AllocEngine constructor and there is
-    // currently no other way to set it, so that check is not really necessary.
-    if (!allocator_) {
-        isc_throw(InvalidOperation, "No allocator selected");
-    }
-
-    // Check if there's existing lease for that subnet/clientid/hwaddr combination.
-    Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(*hwaddr, subnet->getID());
-    if (existing) {
-        // We have a lease already. This is a returning client, probably after
-        // its reboot.
-        existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
-        if (existing) {
-            return (existing);
+    try {
+        // Allocator is always created in AllocEngine constructor and there is
+        // currently no other way to set it, so that check is not really necessary.
+        if (!allocator_) {
+            isc_throw(InvalidOperation, "No allocator selected");
         }
 
-        // If renewal failed (e.g. the lease no longer matches current configuration)
-        // let's continue the allocation process
-    }
-
-    if (clientid) {
-        existing = LeaseMgrFactory::instance().getLease4(*clientid, subnet->getID());
+        // Check if there's existing lease for that subnet/clientid/hwaddr combination.
+        Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(hwaddr->hwaddr_, subnet->getID());
         if (existing) {
-            // we have a lease already. This is a returning client, probably after
+            // We have a lease already. This is a returning client, probably after
             // its reboot.
             existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
-            // @todo: produce a warning. We haven't found him using MAC address, but
-            // we found him using client-id
             if (existing) {
                 return (existing);
             }
+
+            // If renewal failed (e.g. the lease no longer matches current configuration)
+            // let's continue the allocation process
         }
-    }
 
-    // check if the hint is in pool and is available
-    if (subnet->inPool(hint)) {
-        existing = LeaseMgrFactory::instance().getLease4(hint);
-        if (!existing) {
-            /// @todo: Check if the hint is reserved once we have host support
-            /// implemented
+        if (clientid) {
+            existing = LeaseMgrFactory::instance().getLease4(*clientid, subnet->getID());
+            if (existing) {
+                // we have a lease already. This is a returning client, probably after
+                // its reboot.
+                existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
+                // @todo: produce a warning. We haven't found him using MAC address, but
+                // we found him using client-id
+                if (existing) {
+                    return (existing);
+                }
+            }
+        }
 
-            // The hint is valid and not currently used, let's create a lease for it
-            Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, hint, fake_allocation);
+        // check if the hint is in pool and is available
+        if (subnet->inPool(hint)) {
+            existing = LeaseMgrFactory::instance().getLease4(hint);
+            if (!existing) {
+                /// @todo: Check if the hint is reserved once we have host support
+                /// implemented
+
+                // The hint is valid and not currently used, let's create a lease for it
+                Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, hint, fake_allocation);
+
+                // It can happen that the lease allocation failed (we could have lost
+                // the race condition. That means that the hint is lo longer usable and
+                // we need to continue the regular allocation path.
+                if (lease) {
+                    return (lease);
+                }
+            } else {
+                if (existing->expired()) {
+                    return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
+                                              fake_allocation));
+                }
 
-            // It can happen that the lease allocation failed (we could have lost
-            // the race condition. That means that the hint is lo longer usable and
-            // we need to continue the regular allocation path.
-            if (lease) {
-                return (lease);
             }
-        } else {
-            if (existing->expired()) {
-                return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
-                                          fake_allocation));
-            }
-
         }
-    }
 
-    // Hint is in the pool but is not available. Search the pool until first of
-    // the following occurs:
-    // - we find a free address
-    // - we find an address for which the lease has expired
-    // - we exhaust the number of tries
-    //
-    // @todo: Current code does not handle pool exhaustion well. It will be
-    // improved. Current problems:
-    // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
-    // 10 addresses), we will iterate over it 100 times before giving up
-    // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
-    // 3. the whole concept of infinite attempts is just asking for infinite loop
-    // We may consider some form or reference counting (this pool has X addresses
-    // left), but this has one major problem. We exactly control allocation
-    // moment, but we currently do not control expiration time at all
-
-    unsigned int i = attempts_;
-    do {
-        IOAddress candidate = allocator_->pickAddress(subnet, clientid, hint);
-
-        /// @todo: check if the address is reserved once we have host support
-        /// implemented
-
-        Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(candidate);
-        if (!existing) {
-            // there's no existing lease for selected candidate, so it is
-            // free. Let's allocate it.
-            Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, candidate,
-                                          fake_allocation);
-            if (lease) {
-                return (lease);
-            }
+        // Hint is in the pool but is not available. Search the pool until first of
+        // the following occurs:
+        // - we find a free address
+        // - we find an address for which the lease has expired
+        // - we exhaust the number of tries
+        //
+        // @todo: Current code does not handle pool exhaustion well. It will be
+        // improved. Current problems:
+        // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
+        // 10 addresses), we will iterate over it 100 times before giving up
+        // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
+        // 3. the whole concept of infinite attempts is just asking for infinite loop
+        // We may consider some form or reference counting (this pool has X addresses
+        // left), but this has one major problem. We exactly control allocation
+        // moment, but we currently do not control expiration time at all
+
+        unsigned int i = attempts_;
+        do {
+            IOAddress candidate = allocator_->pickAddress(subnet, clientid, hint);
+
+            /// @todo: check if the address is reserved once we have host support
+            /// implemented
 
-            // Although the address was free just microseconds ago, it may have
-            // been taken just now. If the lease insertion fails, we continue
-            // allocation attempts.
-        } else {
-            if (existing->expired()) {
-                return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
-                                          fake_allocation));
+            Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(candidate);
+            if (!existing) {
+                // there's no existing lease for selected candidate, so it is
+                // free. Let's allocate it.
+                Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, candidate,
+                                              fake_allocation);
+                if (lease) {
+                    return (lease);
+                }
+
+                // Although the address was free just microseconds ago, it may have
+                // been taken just now. If the lease insertion fails, we continue
+                // allocation attempts.
+            } else {
+                if (existing->expired()) {
+                    return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
+                                              fake_allocation));
+                }
             }
-        }
 
-        // Continue trying allocation until we run out of attempts
-        // (or attempts are set to 0, which means infinite)
-        --i;
-    } while ( i || !attempts_);
+            // Continue trying allocation until we run out of attempts
+            // (or attempts are set to 0, which means infinite)
+            --i;
+        } while ((i > 0) || !attempts_);
 
-    isc_throw(AllocFailed, "Failed to allocate address after " << attempts_
-              << " tries");
+        // Unable to allocate an address, return an empty lease.
+        LOG_WARN(dhcpsrv_logger, DHCPSRV_ADDRESS4_ALLOC_FAIL).arg(attempts_);
+
+    } catch (const isc::Exception& e) {
+
+        // Some other error, return an empty lease.
+        LOG_ERROR(dhcpsrv_logger, DHCPSRV_ADDRESS4_ALLOC_ERROR).arg(e.what());
+    }
+    return (Lease4Ptr());
 }
 
 Lease4Ptr AllocEngine::renewLease4(const SubnetPtr& subnet,

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -14,6 +14,46 @@
 
 $NAMESPACE isc::dhcp
 
+% DHCPSRV_ADDRESS4_ALLOC_ERROR error during attempt to allocate an IPv4 address: %1
+An error occurred during an attempt to allocate an IPv4 address, the
+reason for the failure being contained in the message.  The server will
+return a message to the client refusing a lease.
+
+% DHCPSRV_ADDRESS4_ALLOC_FAIL failed to allocate an IPv4 address after %1 attempt(s)
+THE DHCP allocation engine gave up trying to allocate an IPv4 address
+after the specified number of attempts.  This probably means that the
+address pool from which the allocation is being attempted is either
+empty, or very nearly empty.  As a result, the client will have been
+refused a lease.
+
+This message may indicate that your address pool is too small for the
+number of clients you are trying to service and should be expanded.
+Alternatively, if the you know that the number of concurrently active
+clients is less than the addresses you have available, you may want to
+consider reducing the lease lifetime.  In this way, addresses allocated
+to clients that are no longer active on the network will become available
+available sooner.
+
+% DHCPSRV_ADDRESS6_ALLOC_ERROR error during attempt to allocate an IPv6 address: %1
+An error occurred during an attempt to allocate an IPv6 address, the
+reason for the failure being contained in the message.  The server will
+return a message to the client refusing a lease.
+
+% DHCPSRV_ADDRESS6_ALLOC_FAIL failed to allocate an IPv6 address after %1 attempt(s)
+The DHCP allocation engine gave up trying to allocate an IPv6 address
+after the specified number of attempts.  This probably means that the
+address pool from which the allocation is being attempted is either
+empty, or very nearly empty.  As a result, the client will have been
+refused a lease.
+
+This message may indicate that your address pool is too small for the
+number of clients you are trying to service and should be expanded.
+Alternatively, if the you know that the number of concurrently active
+clients is less than the addresses you have available, you may want to
+consider reducing the lease lifetime.  In this way, addresses allocated
+to clients that are no longer active on the network will become available
+available sooner.
+
 % DHCPSRV_CFGMGR_ADD_SUBNET4 adding subnet %1
 A debug message reported when the DHCP configuration manager is adding the
 specified IPv4 subnet to its database.

src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -461,9 +461,9 @@ TEST_F(AllocEngine6Test, outOfAddresses6) {
 
     // There is just a single address in the pool and allocated it to someone
     // else, so the allocation should fail
-
-    EXPECT_THROW(engine->allocateAddress6(subnet_, duid_, iaid_, IOAddress("::"),false),
-                 AllocFailed);
+    Lease6Ptr lease2 = engine->allocateAddress6(subnet_, duid_, iaid_,
+                                                IOAddress("::"), false);
+    EXPECT_FALSE(lease2);
 }
 
 // This test checks if an expired lease can be reused in SOLICIT (fake allocation)
@@ -838,8 +838,9 @@ TEST_F(AllocEngine4Test, outOfAddresses4) {
     // There is just a single address in the pool and allocated it to someone
     // else, so the allocation should fail
 
-    EXPECT_THROW(engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),false),
-                 AllocFailed);
+    Lease4Ptr lease2 = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
+                                                IOAddress("0.0.0.0"), false);
+    EXPECT_FALSE(lease2);
 }
 
 // This test checks if an expired lease can be reused in DISCOVER (fake allocation)

src/lib/dns/master_loader.cc

@@ -54,6 +54,7 @@ public:
 
 } // end unnamed namespace
 
+// cppcheck-suppress noConstructor
 class MasterLoader::MasterLoaderImpl {
 public:
     MasterLoaderImpl(const char* master_file,

src/lib/dns/name.cc

@@ -227,7 +227,6 @@ stringParse(Iterator s, Iterator send, bool downcase, Offsets& offsets,
                 isc_throw(BadLabelType,
                           "invalid label type in " << string(orig_s, send));
             }
-            state = ft_escape;
             // FALLTHROUGH
         case ft_escape:
             if (!isdigit(c & 0xff)) {

src/lib/util/io/socketsession.cc

@@ -82,7 +82,7 @@ const size_t INITIAL_BUFSIZE = 512;
 const int SOCKSESSION_BUFSIZE = (DEFAULT_HEADER_BUFLEN + MAX_DATASIZE) * 2;
 
 struct SocketSessionForwarder::ForwarderImpl {
-    ForwarderImpl() : buf_(DEFAULT_HEADER_BUFLEN) {}
+    ForwarderImpl() : fd_(-1), buf_(DEFAULT_HEADER_BUFLEN) {}
     struct sockaddr_un sock_un_;
     socklen_t sock_un_len_;
     int fd_;

tests/tools/perfdhcp/command_options.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -87,7 +87,7 @@ CommandOptions::reset() {
 }
 
 bool
-CommandOptions::parse(int argc, char** const argv) {
+CommandOptions::parse(int argc, char** const argv, bool print_cmd_line) {
     // Reset internal variables used by getopt
     // to eliminate undefined behavior when
     // parsing different command lines multiple times
@@ -125,7 +125,7 @@ CommandOptions::parse(int argc, char** const argv) {
     reset();
 
     // Informs if program has been run with 'h' or 'v' option.
-    bool help_or_version_mode = initialize(argc, argv);
+    bool help_or_version_mode = initialize(argc, argv, print_cmd_line);
     if (!help_or_version_mode) {
         validate();
     }
@@ -133,7 +133,7 @@ CommandOptions::parse(int argc, char** const argv) {
 }
 
 bool
-CommandOptions::initialize(int argc, char** argv) {
+CommandOptions::initialize(int argc, char** argv, bool print_cmd_line) {
     int opt = 0;                // Subsequent options returned by getopt()
     std::string drop_arg;       // Value of -D<value>argument
     size_t percent_loc = 0;     // Location of % sign in -D<value>
@@ -151,10 +151,10 @@ CommandOptions::initialize(int argc, char** argv) {
     // they will be tuned and validated elsewhere
     while((opt = getopt(argc, argv, "hv46r:t:R:b:n:p:d:D:l:P:a:L:"
                         "s:iBc1T:X:O:E:S:I:x:w:")) != -1) {
-        stream << " -" << opt;
+        stream << " -" << static_cast<char>(opt);
         if (optarg) {
             stream << " " << optarg;
-        }  
+        }
         switch (opt) {
         case '1':
             use_first_ = true;
@@ -416,7 +416,9 @@ CommandOptions::initialize(int argc, char** argv) {
         }
     }
 
-    std::cout << "Running: " << stream.str() << std::endl;
+    if (print_cmd_line) {
+        std::cout << "Running: " << stream.str() << std::endl;
+    }
 
     // Handle the local '-l' address/interface
     if (!localname_.empty()) {

tests/tools/perfdhcp/command_options.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -56,9 +56,10 @@ public:
     ///
     /// \param argc Argument count passed to main().
     /// \param argv Argument value array passed to main().
+    /// \param print_cmd_line Print the command line being run to the console.
     /// \throws isc::InvalidParameter if parse fails.
     /// \return true if program has been run in help or version mode ('h' or 'v' flag).
-    bool parse(int argc, char** const argv);
+    bool parse(int argc, char** const argv, bool print_cmd_line = false);
 
     /// \brief Returns IP version.
     ///
@@ -261,9 +262,10 @@ private:
     ///
     /// \param argc Argument count passed to main().
     /// \param argv Argument value array passed to main().
+    /// \param print_cmd_line Print the command line being run to the console.
     /// \throws isc::InvalidParameter if command line options initialization fails.
     /// \return true if program has been run in help or version mode ('h' or 'v' flag).
-    bool initialize(int argc, char** argv);
+    bool initialize(int argc, char** argv, bool print_cmd_line);
 
     /// \brief Validates initialized options.
     ///

tests/tools/perfdhcp/main.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -32,7 +32,11 @@ main(int argc, char* argv[]) {
         // If parser returns true it means that user specified
         // 'h' or 'v' command line option. Program shows the
         // help or version message and exits here.
-        if (command_options.parse(argc, argv)) {
+        // The third argument indicates that the command line
+        // should be printed when it gets parsed. This is useful
+        // in particular when the command line needs to be
+        // extracted from the log file.
+        if (command_options.parse(argc, argv, true)) {
             return (ret_code);
         }
     } catch(isc::Exception& e) {