Index: /trunk/src/VBox/NetworkServices/Dhcpd/ClientId.h
===================================================================
--- /trunk/src/VBox/NetworkServices/Dhcpd/ClientId.h	(revision 79528)
+++ /trunk/src/VBox/NetworkServices/Dhcpd/ClientId.h	(revision 79529)
@@ -57,7 +57,7 @@
     const OptClientId &id() const   { return m_id; }
 
-    /** @name String formatting stuff
+    /** @name String formatting of %R[id].
      * @{ */
-    static void registerFormat(); /* %R[id] */
+    static void registerFormat();
 private:
     static DECLCALLBACK(size_t) rtStrFormat(PFNRTSTROUTPUT pfnOutput, void *pvArgOutput, const char *pszType,
Index: /trunk/src/VBox/NetworkServices/Dhcpd/Db.cpp
===================================================================
--- /trunk/src/VBox/NetworkServices/Dhcpd/Db.cpp	(revision 79528)
+++ /trunk/src/VBox/NetworkServices/Dhcpd/Db.cpp	(revision 79529)
@@ -16,52 +16,38 @@
  */
 
+
+/*********************************************************************************************************************************
+*   Header Files                                                                                                                 *
+*********************************************************************************************************************************/
 #include "DhcpdInternal.h"
 #include <iprt/errcore.h>
-#include <iprt/stream.h>
 
 #include "Db.h"
 
 
-Db::Db()
-  : m_pConfig(NULL)
-{
-    return;
-}
-
-
-Db::~Db()
-{
-    /** @todo free bindings */
-}
-
-
-int Db::init(const Config *pConfig)
-{
-    Binding::registerFormat();
-
-    m_pConfig = pConfig;
-
-    m_pool.init(pConfig->getIPv4PoolFirst(),
-                pConfig->getIPv4PoolLast());
-
-    return VINF_SUCCESS;
-}
-
-
+/*********************************************************************************************************************************
+*   Global Variables                                                                                                             *
+*********************************************************************************************************************************/
+/** Indicates whether has been called successfully yet. */
 bool Binding::g_fFormatRegistered = false;
 
 
+/**
+ * Registers the ClientId format type callback ("%R[binding]").
+ */
 void Binding::registerFormat()
 {
-    if (g_fFormatRegistered)
-        return;
-
-    int rc = RTStrFormatTypeRegister("binding", rtStrFormat, NULL);
-    AssertRC(rc);
-
-    g_fFormatRegistered = true;
-}
-
-
+    if (!g_fFormatRegistered)
+    {
+        int rc = RTStrFormatTypeRegister("binding", rtStrFormat, NULL);
+        AssertRC(rc);
+        g_fFormatRegistered = true;
+    }
+}
+
+
+/**
+ * @callback_method_impl{FNRTSTRFORMATTYPE, Formats ClientId via "%R[binding]".}
+ */
 DECLCALLBACK(size_t)
 Binding::rtStrFormat(PFNRTSTROUTPUT pfnOutput, void *pvArgOutput,
@@ -70,6 +56,4 @@
                      void *pvUser)
 {
-    const Binding *b = static_cast<const Binding *>(pvValue);
-    size_t cb = 0;
 
     AssertReturn(strcmp(pszType, "binding") == 0, 0);
@@ -79,30 +63,19 @@
     RT_NOREF(pvUser);
 
+    const Binding *b = static_cast<const Binding *>(pvValue);
     if (b == NULL)
-    {
-        return RTStrFormat(pfnOutput, pvArgOutput, NULL, 0,
-                           "<NULL>");
-    }
-
-    cb += RTStrFormat(pfnOutput, pvArgOutput, NULL, 0,
-                      "%RTnaipv4", b->m_addr.u);
-
+        return pfnOutput(pvArgOutput, RT_STR_TUPLE("<NULL>"));
+
+    size_t cb = RTStrFormat(pfnOutput, pvArgOutput, NULL, 0, "%RTnaipv4", b->m_addr.u);
     if (b->m_state == Binding::FREE)
-    {
-        cb += RTStrFormat(pfnOutput, pvArgOutput, NULL, 0,
-                          " free");
-    }
+        cb += pfnOutput(pvArgOutput, RT_STR_TUPLE(" free"));
     else
     {
-        cb += RTStrFormat(pfnOutput, pvArgOutput, NULL, 0,
-                          " to %R[id], %s, valid from ",
-                          &b->m_id, b->stateName());
+        cb += RTStrFormat(pfnOutput, pvArgOutput, NULL, 0, " to %R[id], %s, valid from ", &b->m_id, b->stateName());
 
         Timestamp tsIssued = b->issued();
         cb += tsIssued.strFormatHelper(pfnOutput, pvArgOutput);
 
-        cb += RTStrFormat(pfnOutput, pvArgOutput, NULL, 0,
-                          " for %ds until ",
-                          b->leaseTime());
+        cb += RTStrFormat(pfnOutput, pvArgOutput, NULL, 0, " for %ds until ", b->leaseTime());
 
         Timestamp tsValid = b->issued();
@@ -116,17 +89,19 @@
 const char *Binding::stateName() const
 {
-    switch (m_state) {
-    case FREE:
-        return "free";
-    case RELEASED:
-        return "released";
-    case EXPIRED:
-        return "expired";
-    case OFFERED:
-        return "offered";
-    case ACKED:
-        return "acked";
-    default:
-        return "released";
+    switch (m_state)
+    {
+        case FREE:
+            return "free";
+        case RELEASED:
+            return "released";
+        case EXPIRED:
+            return "expired";
+        case OFFERED:
+            return "offered";
+        case ACKED:
+            return "acked";
+        default:
+            AssertMsgFailed(("%d\n", m_state));
+            return "released";
     }
 }
@@ -146,5 +121,8 @@
         m_state = Binding::ACKED;
     else
+    {
+        AssertMsgFailed(("%d\n", m_state));
         m_state = Binding::RELEASED;
+    }
 
     return *this;
@@ -152,13 +130,20 @@
 
 
-bool Binding::expire(Timestamp deadline)
+/**
+ * Expires the binding if it's past the specified deadline.
+ *
+ * @returns False if already expired, released or freed, otherwise true (i.e.
+ *          does not indicate whether action was taken or not).
+ * @param   tsDeadline          The expiry deadline to use.
+ */
+bool Binding::expire(Timestamp tsDeadline)
 {
     if (m_state <= Binding::EXPIRED)
         return false;
 
-    Timestamp t = m_issued;
-    t.addSeconds(m_secLease);
-
-    if (t < deadline)
+    Timestamp tsExpire = m_issued;
+    tsExpire.addSeconds(m_secLease);
+
+    if (tsExpire < tsDeadline)
     {
         if (m_state == Binding::OFFERED)
@@ -171,19 +156,17 @@
 
 
-int Binding::toXML(xml::ElementNode *ndParent) const
-{
-    int rc;
-
+/**
+ * Serializes the binding to XML for the lease database.
+ *
+ * @throw  std::bad_alloc
+ */
+void Binding::toXML(xml::ElementNode *pElmParent) const
+{
     /*
      * Lease
      */
-    xml::ElementNode *ndLease = ndParent->createChild("Lease");
-    if (ndLease == NULL)
-        return VERR_GENERAL_FAILURE;
-
-    /* XXX: arrange for lease to get deleted if anything below fails */
-
-
-    ndLease->setAttribute("mac", RTCStringFmt("%RTmac", &m_id.mac()));
+    xml::ElementNode *pElmLease = pElmParent->createChild("Lease");
+
+    pElmLease->setAttribute("mac", RTCStringFmt("%RTmac", &m_id.mac()));
     if (m_id.id().present())
     {
@@ -191,36 +174,39 @@
         size_t cbStrId = m_id.id().value().size() * 2 + 1;
         char *pszId = new char[cbStrId];
-        rc = RTStrPrintHexBytes(pszId, cbStrId,
-                                &m_id.id().value().front(), m_id.id().value().size(),
-                                0);
-        ndLease->setAttribute("id", pszId);
+        int rc = RTStrPrintHexBytes(pszId, cbStrId,
+                                    &m_id.id().value().front(), m_id.id().value().size(),
+                                    0);
+        AssertRC(rc);
+        pElmLease->setAttribute("id", pszId);
         delete[] pszId;
     }
 
     /* unused but we need it to keep the old code happy */
-    ndLease->setAttribute("network", "0.0.0.0");
-
-    ndLease->setAttribute("state", stateName());
-
+    pElmLease->setAttribute("network", "0.0.0.0");
+    pElmLease->setAttribute("state", stateName());
 
     /*
      * Lease/Address
      */
-    xml::ElementNode *ndAddr = ndLease->createChild("Address");
-    ndAddr->setAttribute("value", RTCStringFmt("%RTnaipv4", m_addr.u));
-
+    xml::ElementNode *pElmAddr = pElmLease->createChild("Address");
+    pElmAddr->setAttribute("value", RTCStringFmt("%RTnaipv4", m_addr.u));
 
     /*
      * Lease/Time
      */
-    xml::ElementNode *ndTime = ndLease->createChild("Time");
-    ndTime->setAttribute("issued", m_issued.getAbsSeconds());
-    ndTime->setAttribute("expiration", m_secLease);
-
-    return VINF_SUCCESS;
-}
-
-
-Binding *Binding::fromXML(const xml::ElementNode *ndLease)
+    xml::ElementNode *pElmTime = pElmLease->createChild("Time");
+    pElmTime->setAttribute("issued", m_issued.getAbsSeconds());
+    pElmTime->setAttribute("expiration", m_secLease);
+}
+
+
+/**
+ * Deserializes the binding from the XML lease database.
+ *
+ * @param   pElmLease   The "Lease" element.
+ * @return  Pointer to the resulting binding, NULL on failure.
+ * @throw   std::bad_alloc
+ */
+Binding *Binding::fromXML(const xml::ElementNode *pElmLease)
 {
     /* Lease/@network seems to always have bogus value, ignore it. */
@@ -230,5 +216,5 @@
      */
     RTCString strMac;
-    bool fHasMac = ndLease->getAttributeValue("mac", &strMac);
+    bool fHasMac = pElmLease->getAttributeValue("mac", &strMac);
     if (!fHasMac)
         return NULL;
@@ -241,5 +227,5 @@
     OptClientId id;
     RTCString strId;
-    bool fHasId = ndLease->getAttributeValue("id", &strId);
+    bool fHasId = pElmLease->getAttributeValue("id", &strId);
     if (fHasId)
     {
@@ -264,10 +250,10 @@
      */
     RTCString strState;
-    bool fHasState = ndLease->getAttributeValue("state", &strState);
+    bool fHasState = pElmLease->getAttributeValue("state", &strState);
 
     /*
      * Lease/Address
      */
-    const xml::ElementNode *ndAddress = ndLease->findChildElement("Address");
+    const xml::ElementNode *ndAddress = pElmLease->findChildElement("Address");
     if (ndAddress == NULL)
         return NULL;
@@ -289,5 +275,5 @@
      * Lease/Time
      */
-    const xml::ElementNode *ndTime = ndLease->findChildElement("Time");
+    const xml::ElementNode *ndTime = pElmLease->findChildElement("Time");
     if (ndTime == NULL)
         return NULL;
@@ -332,10 +318,41 @@
 
 
+
+/*********************************************************************************************************************************
+*   Class Db Implementation                                                                                                      *
+*********************************************************************************************************************************/
+
+Db::Db()
+    : m_pConfig(NULL)
+{
+}
+
+
+Db::~Db()
+{
+    /** @todo free bindings */
+}
+
+
+int Db::init(const Config *pConfig)
+{
+    Binding::registerFormat();
+
+    m_pConfig = pConfig;
+
+    m_pool.init(pConfig->getIPv4PoolFirst(),
+                pConfig->getIPv4PoolLast());
+
+    return VINF_SUCCESS;
+}
+
+
+/**
+ * Expire old binding (leases).
+ */
 void Db::expire()
 {
     const Timestamp now = Timestamp::now();
-
-    for (bindings_t::iterator it = m_bindings.begin();
-         it != m_bindings.end(); ++it)
+    for (bindings_t::iterator it = m_bindings.begin(); it != m_bindings.end(); ++it)
     {
         Binding *b = *it;
@@ -345,16 +362,42 @@
 
 
+/**
+ * Internal worker that creates a binding for the given client, allocating new
+ * IPv4 address for it.
+ *
+ * @returns Pointer to the binding.
+ * @param   id          The client ID.
+ */
 Binding *Db::createBinding(const ClientId &id)
 {
+    Binding      *pBinding = NULL;
     RTNETADDRIPV4 addr = m_pool.allocate();
-    if (addr.u == 0)
-        return NULL;
-
-    Binding *b = new Binding(addr, id);
-    m_bindings.push_front(b);
-    return b;
-}
-
-
+    if (addr.u != 0)
+    {
+        try
+        {
+            pBinding = new Binding(addr, id);
+            m_bindings.push_front(pBinding);
+        }
+        catch (std::bad_alloc &)
+        {
+            if (pBinding)
+                delete pBinding;
+            /** @todo free address (no pool method for that)  */
+        }
+    }
+    return pBinding;
+}
+
+
+/**
+ * Internal worker that creates a binding to the specified IPv4 address for the
+ * given client.
+ *
+ * @returns Pointer to the binding.
+ *          NULL if the address is in use or we ran out of memory.
+ * @param   addr        The IPv4 address.
+ * @param   id          The client.
+ */
 Binding *Db::createBinding(RTNETADDRIPV4 addr, const ClientId &id)
 {
@@ -362,5 +405,5 @@
     if (!fAvailable)
     {
-        /*
+        /** @todo
          * XXX: this should not happen.  If the address is from the
          * pool, which we have verified before, then either it's in
@@ -376,11 +419,11 @@
 
 
+/**
+ * Internal worker that allocates an IPv4 address for the given client, taking
+ * the preferred address (@a addr) into account when possible and if non-zero.
+ */
 Binding *Db::allocateAddress(const ClientId &id, RTNETADDRIPV4 addr)
 {
     Assert(addr.u == 0 || addressBelongs(addr));
-
-    Binding *addrBinding = NULL;
-    Binding *freeBinding = NULL;
-    Binding *reuseBinding = NULL;
 
     if (addr.u != 0)
@@ -394,7 +437,9 @@
      * addresses that can be reused.
      */
-    const Timestamp now = Timestamp::now();
-    for (bindings_t::iterator it = m_bindings.begin();
-         it != m_bindings.end(); ++it)
+    Binding        *addrBinding  = NULL;
+    Binding        *freeBinding  = NULL;
+    Binding        *reuseBinding = NULL;
+    const Timestamp now          = Timestamp::now();
+    for (bindings_t::iterator it = m_bindings.begin(); it != m_bindings.end(); ++it)
     {
         Binding *b = *it;
@@ -460,6 +505,5 @@
             addrBinding = createBinding(addr, id);
             Assert(addrBinding != NULL);
-            LogDHCP(("> .... creating new binding for this address %R[binding]\n",
-                     addrBinding));
+            LogDHCP(("> .... creating new binding for this address %R[binding]\n", addrBinding));
             return addrBinding;
         }
@@ -467,11 +511,9 @@
         if (addrBinding->m_state <= Binding::EXPIRED) /* not in use */
         {
-            LogDHCP(("> .... reusing %s binding for this address\n",
-                     addrBinding->stateName()));
+            LogDHCP(("> .... reusing %s binding for this address\n", addrBinding->stateName()));
             addrBinding->giveTo(id);
             return addrBinding;
         }
-        LogDHCP(("> .... cannot reuse %s binding for this address\n",
-                 addrBinding->stateName()));
+        LogDHCP(("> .... cannot reuse %s binding for this address\n", addrBinding->stateName()));
     }
 
@@ -489,19 +531,16 @@
         idBinding = createBinding();
         if (idBinding != NULL)
-        {
             LogDHCP(("> .... creating new binding\n"));
-        }
         else
         {
             idBinding = reuseBinding;
-            LogDHCP(("> .... reusing %s binding %R[binding]\n",
-                     reuseBinding->stateName(), reuseBinding));
-        }
-    }
-
-    if (idBinding == NULL)
-    {
-        LogDHCP(("> .... failed to allocate binding\n"));
-        return NULL;
+            if (idBinding != NULL)
+                LogDHCP(("> .... reusing %s binding %R[binding]\n", reuseBinding->stateName(), reuseBinding));
+            else
+            {
+                LogDHCP(("> .... failed to allocate binding\n"));
+                return NULL;
+            }
+        }
     }
 
@@ -514,7 +553,17 @@
 
 
+/**
+ * Called by DHCPD to allocate a binding for the specified request.
+ *
+ * @returns Pointer to the binding, NULL on failure.
+ * @param   req                 The DHCP request being served.
+ */
 Binding *Db::allocateBinding(const DhcpClientMessage &req)
 {
     /** @todo XXX: handle fixed address assignments */
+
+    /*
+     * Get and validate the requested address (if present).
+     */
     OptRequestedAddress reqAddr(req);
     if (reqAddr.present() && !addressBelongs(reqAddr.value()))
@@ -532,63 +581,85 @@
     }
 
+    /*
+     * Allocate the address.
+     */
     const ClientId &id(req.clientId());
 
     Binding *b = allocateAddress(id, reqAddr.value());
-    if (b == NULL)
-        return NULL;
-
-    Assert(b->id() == id);
-
-    /** @todo
-     * XXX: handle requests for specific lease time!
-     * XXX: old lease might not have expired yet?
-     */
-    // OptLeaseTime reqLeaseTime(req);
-    b->setLeaseTime(1200);
+    if (b != NULL)
+    {
+        Assert(b->id() == id);
+
+        /** @todo
+         * XXX: handle requests for specific lease time!
+         * XXX: old lease might not have expired yet?
+         * Make lease time configurable.
+         */
+        // OptLeaseTime reqLeaseTime(req);
+        b->setLeaseTime(1200);
+    }
     return b;
 }
 
 
-int Db::addBinding(Binding *newb)
-{
-    if (!addressBelongs(newb->m_addr))
-    {
-        LogDHCP(("Binding for out of range address %RTnaipv4 ignored\n",
-                 newb->m_addr.u));
-        return VERR_INVALID_PARAMETER;
-    }
-
-    for (bindings_t::iterator it = m_bindings.begin();
-         it != m_bindings.end(); ++it)
+/**
+ * Internal worker used by loadLease().
+ *
+ * @returns IPRT status code.
+ * @param   newb                .
+ */
+int Db::addBinding(Binding *pNewBinding)
+{
+    /*
+     * Validate the binding against the range and existing bindings.
+     */
+    if (!addressBelongs(pNewBinding->m_addr))
+    {
+        LogDHCP(("Binding for out of range address %RTnaipv4 ignored\n", pNewBinding->m_addr.u));
+        return VERR_OUT_OF_RANGE;
+    }
+
+    for (bindings_t::iterator it = m_bindings.begin(); it != m_bindings.end(); ++it)
     {
         Binding *b = *it;
 
-        if (newb->m_addr.u == b->m_addr.u)
-        {
-            LogDHCP(("> ADD: %R[binding]\n", newb));
+        if (pNewBinding->m_addr.u == b->m_addr.u)
+        {
+            LogDHCP(("> ADD: %R[binding]\n", pNewBinding));
             LogDHCP(("> .... duplicate ip: %R[binding]\n", b));
-            return VERR_INVALID_PARAMETER;
-        }
-
-        if (newb->m_id == b->m_id)
-        {
-            LogDHCP(("> ADD: %R[binding]\n", newb));
+            return VERR_DUPLICATE;
+        }
+
+        if (pNewBinding->m_id == b->m_id)
+        {
+            LogDHCP(("> ADD: %R[binding]\n", pNewBinding));
             LogDHCP(("> .... duplicate id: %R[binding]\n", b));
-            return VERR_INVALID_PARAMETER;
-        }
-    }
-
-    bool ok = m_pool.allocate(newb->m_addr);
-    if (!ok)
-    {
-        LogDHCP(("> ADD: failed to claim IP %R[binding]\n", newb));
-        return VERR_INVALID_PARAMETER;
-    }
-
-    m_bindings.push_back(newb);
+            return VERR_DUPLICATE;
+        }
+    }
+
+    /*
+     * Allocate the address and add the binding to the list.
+     */
+    AssertLogRelMsgReturn(m_pool.allocate(pNewBinding->m_addr),
+                          ("> ADD: failed to claim IP %R[binding]\n", pNewBinding),
+                          VERR_INTERNAL_ERROR);
+    try
+    {
+        m_bindings.push_back(pNewBinding);
+    }
+    catch (std::bad_alloc &)
+    {
+        return VERR_NO_MEMORY;
+    }
     return VINF_SUCCESS;
 }
 
 
+/**
+ * Called by DHCP to cancel an offset.
+ *
+ * @param   req                 The DHCP request.
+ */
 void Db::cancelOffer(const DhcpClientMessage &req)
 {
@@ -598,8 +669,7 @@
 
     const RTNETADDRIPV4 addr = reqAddr.value();
-    const ClientId &id(req.clientId());
-
-    for (bindings_t::iterator it = m_bindings.begin();
-         it != m_bindings.end(); ++it)
+    const ClientId     &id(req.clientId());
+
+    for (bindings_t::iterator it = m_bindings.begin(); it != m_bindings.end(); ++it)
     {
         Binding *b = *it;
@@ -609,20 +679,29 @@
             if (b->state() == Binding::OFFERED)
             {
+                LogRel2(("Db::cancelOffer: cancelling %R[binding]\n", b));
                 b->setLeaseTime(0);
                 b->setState(Binding::RELEASED);
             }
+            else
+                LogRel2(("Db::cancelOffer: not offered state: %R[binding]\n", b));
             return;
         }
     }
-}
-
-
+    LogRel2(("Db::cancelOffer: not found (%RTnaipv4, %R[id])\n", addr.u, &id));
+}
+
+
+/**
+ * Called by DHCP to cancel an offset.
+ *
+ * @param   req                 The DHCP request.
+ * @returns true if found and released, otherwise false.
+ */
 bool Db::releaseBinding(const DhcpClientMessage &req)
 {
     const RTNETADDRIPV4 addr = req.ciaddr();
-    const ClientId &id(req.clientId());
-
-    for (bindings_t::iterator it = m_bindings.begin();
-         it != m_bindings.end(); ++it)
+    const ClientId     &id(req.clientId());
+
+    for (bindings_t::iterator it = m_bindings.begin(); it != m_bindings.end(); ++it)
     {
         Binding *b = *it;
@@ -630,4 +709,5 @@
         if (b->addr().u == addr.u && b->id() == id)
         {
+            LogRel2(("Db::releaseBinding: releasing %R[binding]\n", b));
             b->setState(Binding::RELEASED);
             return true;
@@ -635,30 +715,50 @@
     }
 
+    LogRel2(("Db::releaseBinding: not found (%RTnaipv4, %R[id])\n", addr.u, &id));
     return false;
 }
 
 
-int Db::writeLeases(const RTCString &strFileName) const
-{
-    LogDHCP(("writing leases to %s\n", strFileName.c_str()));
-
+/**
+ * Called by DHCPD to write out the lease database to @a strFilename.
+ *
+ * @returns IPRT status code.
+ * @param   strFilename         The file to write it to.
+ */
+int Db::writeLeases(const RTCString &strFilename) const
+{
+    LogDHCP(("writing leases to %s\n", strFilename.c_str()));
+
+    /*
+     * Create the document and root element.
+     */
     xml::Document doc;
-
-    xml::ElementNode *root = doc.createRootElement("Leases");
-    if (root == NULL)
-        return VERR_INTERNAL_ERROR;
-
-    root->setAttribute("version", "1.0");
-
-    for (bindings_t::const_iterator it = m_bindings.begin();
-         it != m_bindings.end(); ++it)
-    {
-        const Binding *b = *it;
-        b->toXML(root);
-    }
-
-    try {
+    try
+    {
+        xml::ElementNode *pElmRoot = doc.createRootElement("Leases");
+        pElmRoot->setAttribute("version", "1.0");
+
+        /*
+         * Add the leases.
+         */
+        for (bindings_t::const_iterator it = m_bindings.begin(); it != m_bindings.end(); ++it)
+        {
+            const Binding *b = *it;
+            b->toXML(pElmRoot);
+        }
+    }
+    catch (std::bad_alloc &)
+    {
+        return VERR_NO_MEMORY;
+    }
+
+    /*
+     * Write the document to the specified file in a safe manner (written to temporary
+     * file, renamed to destination on success)
+     */
+    try
+    {
         xml::XmlFileWriter writer(doc);
-        writer.write(strFileName.c_str(), true);
+        writer.write(strFilename.c_str(), true /*fSafe*/);
     }
     catch (const xml::EIPRTFailure &e)
@@ -674,7 +774,6 @@
     catch (...)
     {
-        LogDHCP(("Unknown exception while writing '%s'\n",
-                 strFileName.c_str()));
-        return VERR_GENERAL_FAILURE;
+        LogDHCP(("Unknown exception while writing '%s'\n", strFilename.c_str()));
+        return VERR_UNEXPECTED_EXCEPTION;
     }
 
@@ -683,13 +782,24 @@
 
 
-int Db::loadLeases(const RTCString &strFileName)
-{
-    LogDHCP(("loading leases from %s\n", strFileName.c_str()));
-
+/**
+ * Called by DHCPD to load the lease database to @a strFilename.
+ *
+ * @note Does not clear the database state before doing the load.
+ *
+ * @returns IPRT status code.
+ * @param   strFilename         The file to load it from.
+ */
+int Db::loadLeases(const RTCString &strFilename)
+{
+    LogDHCP(("loading leases from %s\n", strFilename.c_str()));
+
+    /*
+     * Load the file into an XML document.
+     */
     xml::Document doc;
     try
     {
         xml::XmlFileParser parser;
-        parser.read(strFileName.c_str(), doc);
+        parser.read(strFilename.c_str(), doc);
     }
     catch (const xml::EIPRTFailure &e)
@@ -705,39 +815,77 @@
     catch (...)
     {
-        LogDHCP(("Unknown exception while reading and parsing '%s'\n",
-                 strFileName.c_str()));
-        return VERR_GENERAL_FAILURE;
-    }
-
-    xml::ElementNode *ndRoot = doc.getRootElement();
-    if (ndRoot == NULL || !ndRoot->nameEquals("Leases"))
-    {
+        LogDHCP(("Unknown exception while reading and parsing '%s'\n", strFilename.c_str()));
+        return VERR_UNEXPECTED_EXCEPTION;
+    }
+
+    /*
+     * Check that the root element is "Leases" and process its children.
+     */
+    xml::ElementNode *pElmRoot = doc.getRootElement();
+    if (!pElmRoot)
+    {
+        LogDHCP(("No root element in '%s'\n", strFilename.c_str()));
         return VERR_NOT_FOUND;
     }
-
-    xml::NodesLoop it(*ndRoot);
-    const xml::ElementNode *node;
-    while ((node = it.forAllNodes()) != NULL)
-    {
-        if (!node->nameEquals("Lease"))
-            continue;
-
-        loadLease(node);
-    }
-
-    return VINF_SUCCESS;
-}
-
-
-void Db::loadLease(const xml::ElementNode *ndLease)
-{
-    Binding *b = Binding::fromXML(ndLease);
-    bool expired = b->expire();
-
-    if (!expired)
-        LogDHCP(("> LOAD: lease %R[binding]\n", b));
-    else
-        LogDHCP(("> LOAD: EXPIRED lease %R[binding]\n", b));
-
-    addBinding(b);
-}
+    if (!pElmRoot->nameEquals("Leases"))
+    {
+        LogDHCP(("No root element is not 'Leases' in '%s', but '%s'\n", strFilename.c_str(), pElmRoot->getName()));
+        return VERR_NOT_FOUND;
+    }
+
+    int                     rc = VINF_SUCCESS;
+    xml::NodesLoop          it(*pElmRoot);
+    const xml::ElementNode *pElmLease;
+    while ((pElmLease = it.forAllNodes()) != NULL)
+    {
+        if (pElmLease->nameEquals("Lease"))
+        {
+            int rc2 = loadLease(pElmLease);
+            if (RT_SUCCESS(rc2))
+            { /* likely */ }
+            else if (rc2 == VERR_NO_MEMORY)
+                return rc2;
+            else
+                rc = -rc2;
+        }
+        else
+            LogDHCP(("Ignoring unexpected element '%s' under 'Leases'...\n", pElmLease->getName()));
+    }
+
+    return rc;
+}
+
+
+/**
+ * Internal worker for loadLeases() that handles one 'Lease' element.
+ *
+ * @param   pElmLease           The 'Lease' element to handle.
+ * @return  IPRT status code.
+ */
+int Db::loadLease(const xml::ElementNode *pElmLease)
+{
+    Binding *pBinding = NULL;
+    try
+    {
+        pBinding = Binding::fromXML(pElmLease);
+    }
+    catch (std::bad_alloc &)
+    {
+        return VERR_NO_MEMORY;
+    }
+    if (pBinding)
+    {
+        bool fExpired = pBinding->expire();
+        if (!fExpired)
+            LogDHCP(("> LOAD: lease %R[binding]\n", pBinding));
+        else
+            LogDHCP(("> LOAD: EXPIRED lease %R[binding]\n", pBinding));
+
+        int rc = addBinding(pBinding);
+        if (RT_FAILURE(rc))
+            delete pBinding;
+        return rc;
+    }
+    LogDHCP(("> LOAD: failed to load lease!\n"));
+    return VERR_PARSE_ERROR;
+}
Index: /trunk/src/VBox/NetworkServices/Dhcpd/Db.h
===================================================================
--- /trunk/src/VBox/NetworkServices/Dhcpd/Db.h	(revision 79528)
+++ /trunk/src/VBox/NetworkServices/Dhcpd/Db.h	(revision 79529)
@@ -37,4 +37,7 @@
 
 
+/**
+ * Address binding in the lease database.
+ */
 class Binding
 {
@@ -55,30 +58,32 @@
     Binding(const Binding &);
 
-    explicit Binding(RTNETADDRIPV4 addrParam)
-      : m_addr(addrParam), m_state(FREE),
-        m_issued(), m_secLease() {}
+    explicit Binding(RTNETADDRIPV4 a_Addr)
+        : m_addr(a_Addr), m_state(FREE), m_issued(), m_secLease()
+    {}
 
-    Binding(RTNETADDRIPV4 addrParam, const ClientId &idParam)
-      : m_addr(addrParam), m_state(FREE), m_id(idParam),
-        m_issued(), m_secLease() {}
+    Binding(RTNETADDRIPV4 a_Addr, const ClientId &a_id)
+        : m_addr(a_Addr), m_state(FREE), m_id(a_id), m_issued(), m_secLease()
+    {}
 
 
-    RTNETADDRIPV4 addr() const { return m_addr; }
+    /** @name Attribute accessors
+     * @{ */
+    RTNETADDRIPV4   addr() const        { return m_addr; }
 
-    State state() const { return m_state; }
-    const char *stateName() const;
+    const ClientId &id() const          { return m_id; }
 
-    const ClientId &id() const { return m_id; }
+    uint32_t        leaseTime() const   { return m_secLease; }
+    Timestamp       issued() const      { return m_issued; }
 
-    uint32_t leaseTime() const { return m_secLease; }
-    Timestamp issued() const { return m_issued; }
-
-    Binding &setState(State stateParam)
+    State           state() const       { return m_state; }
+    const char     *stateName() const;
+    Binding        &setState(const char *pszStateName);
+    Binding        &setState(State stateParam)
     {
         m_state = stateParam;
         return *this;
     }
+    /** @} */
 
-    Binding &setState(const char *pszStateName);
 
     Binding &setLeaseTime(uint32_t secLease)
@@ -89,7 +94,8 @@
     }
 
-    Binding &giveTo(const ClientId &idParam)
+    /** Reassigns the binding to the given client.   */
+    Binding &giveTo(const ClientId &a_id)
     {
-        m_id = idParam;
+        m_id = a_id;
         m_state = FREE;
         return *this;
@@ -102,23 +108,27 @@
     }
 
-    bool expire(Timestamp deadline);
+    bool expire(Timestamp tsDeadline);
     bool expire() { return expire(Timestamp::now()); }
 
-    static Binding *fromXML(const xml::ElementNode *ndLease);
-    int toXML(xml::ElementNode *ndParent) const;
+    /** @name Serialization
+     * @{ */
+    static Binding *fromXML(const xml::ElementNode *pElmLease);
+    void            toXML(xml::ElementNode *pElmParent) const;
+    /** @} */
 
-public:
-    static void registerFormat(); /* %R[binding] */
-
+    /** @name String formatting of %R[binding].
+     * @{ */
+    static void registerFormat();
 private:
+    static DECLCALLBACK(size_t) rtStrFormat(PFNRTSTROUTPUT pfnOutput, void *pvArgOutput, const char *pszType,
+                                            void const *pvValue, int cchWidth, int cchPrecision, unsigned fFlags, void *pvUser);
     static bool g_fFormatRegistered;
-    static DECLCALLBACK(size_t) rtStrFormat(
-        PFNRTSTROUTPUT pfnOutput, void *pvArgOutput,
-        const char *pszType, void const *pvValue,
-        int cchWidth, int cchPrecision, unsigned fFlags,
-        void *pvUser);
+    /** @} */
 };
 
 
+/**
+ * The lease database.
+ */
 class Db
 {
@@ -126,7 +136,10 @@
     typedef std::list<Binding *> bindings_t;
 
-    const Config *m_pConfig;
-    bindings_t m_bindings;
-    IPv4Pool m_pool;
+    /** Configuration (set at init). */
+    const Config   *m_pConfig;
+    /** The lease database. */
+    bindings_t      m_bindings;
+    /** Address allocation pool. */
+    IPv4Pool        m_pool;
 
 public:
@@ -134,20 +147,24 @@
     ~Db();
 
-    int init(const Config *pConfig);
+    int      init(const Config *pConfig);
 
-    bool addressBelongs(RTNETADDRIPV4 addr) const { return m_pool.contains(addr); }
+    /** Check if @a addr belonges to this lease database. */
+    bool     addressBelongs(RTNETADDRIPV4 addr) const { return m_pool.contains(addr); }
 
     Binding *allocateBinding(const DhcpClientMessage &req);
-    bool releaseBinding(const DhcpClientMessage &req);
+    bool     releaseBinding(const DhcpClientMessage &req);
 
-    void cancelOffer(const DhcpClientMessage &req);
+    void     cancelOffer(const DhcpClientMessage &req);
 
-    void expire();
+    void     expire();
 
+    /** @name Database serialization methods
+     * @{ */
+    int      loadLeases(const RTCString &strFilename);
+private:
+    int      loadLease(const xml::ElementNode *pElmLease);
 public:
-    int loadLeases(const RTCString &strFileName);
-    void loadLease(const xml::ElementNode *ndLease);
-
-    int writeLeases(const RTCString &strFileName) const;
+    int      writeLeases(const RTCString &strFilename) const;
+    /** @} */
 
 private:
@@ -158,5 +175,5 @@
 
     /* add binding e.g. from the leases file */
-    int addBinding(Binding *b);
+    int      addBinding(Binding *b);
 };
 
Index: /trunk/src/VBox/NetworkServices/Dhcpd/IPv4Pool.cpp
===================================================================
--- /trunk/src/VBox/NetworkServices/Dhcpd/IPv4Pool.cpp	(revision 79528)
+++ /trunk/src/VBox/NetworkServices/Dhcpd/IPv4Pool.cpp	(revision 79529)
@@ -1,5 +1,5 @@
 /* $Id$ */
 /** @file
- * DHCP server - a pool of IPv4 addresses
+ * DHCP server - A pool of IPv4 addresses.
  */
 
@@ -16,7 +16,10 @@
  */
 
+
+/*********************************************************************************************************************************
+*   Header Files                                                                                                                 *
+*********************************************************************************************************************************/
 #include "DhcpdInternal.h"
 #include <iprt/errcore.h>
-#include <iprt/stream.h>
 
 #include "IPv4Pool.h"
@@ -25,6 +28,5 @@
 int IPv4Pool::init(const IPv4Range &aRange)
 {
-    if (!aRange.isValid())
-        return VERR_INVALID_PARAMETER;
+    AssertReturn(aRange.isValid(), VERR_INVALID_PARAMETER);
 
     m_range = aRange;
@@ -36,104 +38,131 @@
 int IPv4Pool::init(RTNETADDRIPV4 aFirstAddr, RTNETADDRIPV4 aLastAddr)
 {
-    IPv4Range range(aFirstAddr, aLastAddr);
+    return init(IPv4Range(aFirstAddr, aLastAddr));
+}
 
-    if (!range.isValid())
-        return VERR_INVALID_PARAMETER;
 
-    m_range = range;
-    m_pool.insert(m_range);
+/**
+ * Internal worker for inserting a range into the pool of available addresses.
+ *
+ * @returns IPRT status code (asserted).
+ * @param   a_Range         The range to insert.
+ */
+int IPv4Pool::insert(const IPv4Range &a_Range)
+{
+    /*
+     * Check preconditions. Asserting because nobody checks the return code.
+     */
+    AssertReturn(m_range.isValid(), VERR_INVALID_STATE);
+    AssertReturn(a_Range.isValid(), VERR_INVALID_PARAMETER);
+    AssertReturn(m_range.contains(a_Range), VERR_INVALID_PARAMETER);
+
+    /*
+     * Check that the incoming range doesn't overlap with existing ranges in the pool.
+     */
+    it_t itHint = m_pool.upper_bound(IPv4Range(a_Range.LastAddr)); /* successor, insertion hint */
+#if 0 /** @todo r=bird: This code is wrong.  It has no end() check for starters.  Since the method is
+       *                only for internal consumption, I've replaced it with a strict build assertion. */
+    if (itHint != m_pool.begin())
+    {
+        it_t prev(itHint);
+        --prev;
+        if (a_Range.FirstAddr <= prev->LastAddr)
+        {
+            LogDHCP(("%08x-%08x conflicts with %08x-%08x\n",
+                     a_Range.FirstAddr, a_Range.LastAddr,
+                     prev->FirstAddr, prev->LastAddr));
+            return VERR_INVALID_PARAMETER;
+        }
+    }
+#endif
+#ifdef VBOX_STRICT
+    for (it_t it2 = m_pool.begin(); it2 != m_pool.end(); ++it2)
+        AssertMsg(it2->LastAddr < a_Range.FirstAddr || it2->FirstAddr > a_Range.LastAddr,
+                  ("%08RX32-%08RX32 conflicts with %08RX32-%08RX32\n",
+                   a_Range.FirstAddr, a_Range.LastAddr, it2->FirstAddr, it2->LastAddr));
+#endif
+
+    /*
+     * No overlaps, insert it.
+     */
+    m_pool.insert(itHint, a_Range);
     return VINF_SUCCESS;
 }
 
 
-int IPv4Pool::insert(const IPv4Range &range)
+/**
+ * Allocates an available IPv4 address from the pool.
+ *
+ * @returns Non-zero network order IPv4 address on success, zero address
+ *          (0.0.0.0) on failure.
+ */
+RTNETADDRIPV4 IPv4Pool::allocate()
 {
-    if (!m_range.isValid())
-        return VERR_INVALID_PARAMETER;
+    RTNETADDRIPV4 RetAddr;
+    if (!m_pool.empty())
+    {
+        /* Grab the first address in the pool: */
+        it_t itBeg = m_pool.begin();
+        RetAddr.u = RT_H2N_U32(itBeg->FirstAddr);
 
-    if (!m_range.contains(range))
-        return VERR_INVALID_PARAMETER;
-
-    it_t it = m_pool.upper_bound(IPv4Range(range.LastAddr)); /* successor */
-    if (it != m_pool.begin())
-    {
-        it_t prev(it);
-        --prev;
-        if (range.FirstAddr <= prev->LastAddr) {
-#if 1 /* XXX */
-            RTPrintf("%08x-%08x conflicts with %08x-%08x\n",
-                     range.FirstAddr, range.LastAddr,
-                     prev->FirstAddr, prev->LastAddr);
-#endif
-            return VERR_INVALID_PARAMETER;
+        if (itBeg->FirstAddr == itBeg->LastAddr)
+            m_pool.erase(itBeg);
+        else
+        {
+            /* Trim the entry (re-inserting it): */
+            IPv4Range trimmed = *itBeg;
+            trimmed.FirstAddr += 1;
+            m_pool.erase(itBeg);
+            m_pool.insert(trimmed);
         }
     }
-
-    m_pool.insert(it, range);
-    return VINF_SUCCESS;
+    else
+        RetAddr.u = 0;
+    return RetAddr;
 }
 
 
-RTNETADDRIPV4 IPv4Pool::allocate()
+/**
+ * Allocate the given address.
+ *
+ * @returns Success indicator.
+ * @param   a_Addr      The IP address to allocate (network order).
+ */
+bool IPv4Pool::allocate(RTNETADDRIPV4 a_Addr)
 {
-    if (m_pool.empty())
+    /*
+     * Find the range containing a_Addr.
+     */
+    it_t it = m_pool.lower_bound(IPv4Range(a_Addr)); /* candidate range */
+    if (it != m_pool.end())
     {
-        RTNETADDRIPV4 res = { 0 };
-        return res;
-    }
+        Assert(RT_N2H_U32(a_Addr.u) <= it->LastAddr); /* by definition of < and lower_bound */
 
-    it_t beg = m_pool.begin();
-    ip_haddr_t addr = beg->FirstAddr;
+        if (it->contains(a_Addr))
+        {
+            /*
+             * Remove a_Addr from the range by way of re-insertion.
+             */
+            const IPV4HADDR haddr = RT_N2H_U32(a_Addr.u);
+            IPV4HADDR       first = it->FirstAddr;
+            IPV4HADDR       last  = it->LastAddr;
 
-    if (beg->FirstAddr == beg->LastAddr)
-    {
-        m_pool.erase(beg);
-    }
-    else
-    {
-        IPv4Range trimmed = *beg;
-        ++trimmed.FirstAddr;
-        m_pool.erase(beg);
-        m_pool.insert(trimmed);
-    }
+            m_pool.erase(it);
+            if (first != last)
+            {
+                if (haddr == first)
+                    insert(++first, last);
+                else if (haddr == last)
+                    insert(first, --last);
+                else
+                {
+                    insert(first, haddr - 1);
+                    insert(haddr + 1, last);
+                }
+            }
 
-    RTNETADDRIPV4 res = { RT_H2N_U32(addr) };
-    return res;
-}
-
-
-bool IPv4Pool::allocate(RTNETADDRIPV4 addr)
-{
-    it_t it = m_pool.lower_bound(IPv4Range(addr)); /* candidate range */
-    if (it == m_pool.end())
-        return false;
-
-    Assert(RT_N2H_U32(addr.u) <= it->LastAddr); /* by definition of < and lower_bound */
-
-    if (!it->contains(addr))
-        return false;
-
-    const ip_haddr_t haddr = RT_N2H_U32(addr.u);
-    ip_haddr_t first = it->FirstAddr;
-    ip_haddr_t last = it->LastAddr;
-
-    m_pool.erase(it);
-    if (first != last)
-    {
-        if (haddr == first)
-        {
-            insert(++first, last);
-        }
-        else if (haddr == last)
-        {
-            insert(first, --last);
-        }
-        else
-        {
-            insert(first, haddr - 1);
-            insert(haddr + 1, last);
+            return true;
         }
     }
-
-    return true;
+    return false;
 }
Index: /trunk/src/VBox/NetworkServices/Dhcpd/IPv4Pool.h
===================================================================
--- /trunk/src/VBox/NetworkServices/Dhcpd/IPv4Pool.h	(revision 79528)
+++ /trunk/src/VBox/NetworkServices/Dhcpd/IPv4Pool.h	(revision 79529)
@@ -27,29 +27,36 @@
 #include <set>
 
-typedef uint32_t ip_haddr_t;    /* in host order */
+
+/** Host order IPv4 address. */
+typedef uint32_t IPV4HADDR;
 
 
-/*
+/**
  * A range of IPv4 addresses (in host order).
  */
 struct IPv4Range
 {
-    ip_haddr_t FirstAddr;
-    ip_haddr_t LastAddr;        /* inclusive */
+    IPV4HADDR FirstAddr;       /**< Lowest address. */
+    IPV4HADDR LastAddr;        /**< Higest address (inclusive). */
 
     IPv4Range()
-      : FirstAddr(), LastAddr() {}
+        : FirstAddr(0), LastAddr(0)
+    {}
 
-    explicit IPv4Range(ip_haddr_t aSingleAddr)
-      : FirstAddr(aSingleAddr), LastAddr(aSingleAddr) {}
+    explicit IPv4Range(IPV4HADDR aSingleAddr)
+        : FirstAddr(aSingleAddr), LastAddr(aSingleAddr)
+    {}
 
-    IPv4Range(ip_haddr_t aFirstAddr, ip_haddr_t aLastAddr)
-      : FirstAddr(aFirstAddr), LastAddr(aLastAddr) {}
+    IPv4Range(IPV4HADDR aFirstAddr, IPV4HADDR aLastAddr)
+        : FirstAddr(aFirstAddr), LastAddr(aLastAddr)
+    {}
 
     explicit IPv4Range(RTNETADDRIPV4 aSingleAddr)
-      : FirstAddr(RT_N2H_U32(aSingleAddr.u)), LastAddr(RT_N2H_U32(aSingleAddr.u)) {}
+        : FirstAddr(RT_N2H_U32(aSingleAddr.u)), LastAddr(RT_N2H_U32(aSingleAddr.u))
+    {}
 
     IPv4Range(RTNETADDRIPV4 aFirstAddr, RTNETADDRIPV4 aLastAddr)
-      : FirstAddr(RT_N2H_U32(aFirstAddr.u)), LastAddr(RT_N2H_U32(aLastAddr.u)) {}
+        : FirstAddr(RT_N2H_U32(aFirstAddr.u)), LastAddr(RT_N2H_U32(aLastAddr.u))
+    {}
 
     bool isValid() const
@@ -58,5 +65,5 @@
     }
 
-    bool contains(ip_haddr_t addr) const
+    bool contains(IPV4HADDR addr) const
     {
         return FirstAddr <= addr && addr <= LastAddr;
@@ -68,7 +75,10 @@
     }
 
-    bool contains(const IPv4Range &range) const
+    /** Checks if this range includes the @a a_rRange. */
+    bool contains(const IPv4Range &a_rRange) const
     {
-        return range.isValid() && FirstAddr <= range.FirstAddr && range.LastAddr <= LastAddr;
+        return a_rRange.isValid()
+            && FirstAddr <= a_rRange.FirstAddr
+            && a_rRange.LastAddr <= LastAddr;
     }
 };
@@ -87,4 +97,11 @@
 
 
+/**
+ * IPv4 address pool.
+ *
+ * This manages a single range of IPv4 addresses (m_range).   Unallocated
+ * addresses are tracked as a set of sub-ranges in the m_pool set.
+ *
+ */
 class IPv4Pool
 {
@@ -92,34 +109,35 @@
     typedef set_t::iterator it_t;
 
-    IPv4Range m_range;
-    set_t m_pool;
+    /** The IPv4 range of this pool. */
+    IPv4Range   m_range;
+    /** Pool of available IPv4 ranges. */
+    set_t       m_pool;
 
 public:
-    IPv4Pool() {}
+    IPv4Pool()
+    {}
 
     int init(const IPv4Range &aRange);
     int init(RTNETADDRIPV4 aFirstAddr, RTNETADDRIPV4 aLastAddr);
 
+    /**
+     * Checks if the pool range includes @a addr (allocation status not considered).
+     */
     bool contains(RTNETADDRIPV4 addr) const
-      { return m_range.contains(addr); }
-
-    int insert(const IPv4Range &range);
-
-#if 0
-    int insert(ip_haddr_t single)
-      { return insert(IPv4Range(single)); }
-#endif
-
-    int insert(ip_haddr_t first, ip_haddr_t last)
-      { return insert(IPv4Range(first, last)); }
-
-    int insert(RTNETADDRIPV4 single)
-      { return insert(IPv4Range(single)); }
-
-    int insert(RTNETADDRIPV4 first, RTNETADDRIPV4 last)
-      { return insert(IPv4Range(first, last)); }
+    {
+        return m_range.contains(addr);
+    }
 
     RTNETADDRIPV4 allocate();
-    bool allocate(RTNETADDRIPV4);
+    bool          allocate(RTNETADDRIPV4);
+
+private:
+    int insert(const IPv4Range &range);
+#if 0
+    int insert(IPV4HADDR single)                            { return insert(IPv4Range(single)); }
+#endif
+    int insert(IPV4HADDR first, IPV4HADDR last)             { return insert(IPv4Range(first, last)); }
+    int insert(RTNETADDRIPV4 single)                        { return insert(IPv4Range(single)); }
+    int insert(RTNETADDRIPV4 first, RTNETADDRIPV4 last)     { return insert(IPv4Range(first, last)); }
 };
 
