Cleanup: use new/delete allocation from netdbEntry class

Use the C++ new/delete operators provided by MEMPRXY_CLASS and
associated as-needed pool creation instead of C Alloc/Free
functions and pre-initialized pool.

Use class initializers for members to demonstrate mixed default
and explicit member initialization.

Add unit test for the new constructor logic.

diff --git a/src/Makefile.am b/src/Makefile.am
index 41f16efd8839e09f5bb0205b0f9f343fd269b42b..4c132c9f70931fe87ee18f4ce2ffe7cf1d11482c 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -879,6 +879,7 @@ check_PROGRAMS+=\
        tests/testHttpRequest \
        tests/testIcmp \
        tests/testIpAddress \
+       tests/testNetDb \
        tests/testStore \
        tests/testString \
        tests/testURL \
@@ -2666,6 +2667,25 @@ tests_testIcmp_LDADD=\
        $(COMPAT_LIB) \
        $(XTRA_LIBS)
 
+tests_testNetDb_SOURCES = \
+       tests/testNetDb.cc \
+       tests/testNetDb.h
+nodist_tests_testNetDb_SOURCES = \
+       SquidTime.h \
+       tests/stub_debug.cc \
+       tests/stub_libmem.cc \
+       time.cc \
+       globals.cc
+tests_testNetDb_LDFLAGS = $(LIBADD_DL)
+tests_testNetDb_LDADD = \
+       icmp/libicmp.la \
+       ip/libip.la \
+       base/libbase.la \
+       $(top_builddir)/lib/libmisccontainers.la \
+       $(LIBCPPUNIT_LIBS) \
+       $(COMPAT_LIB) \
+       $(XTRA_LIBS)
+
 ## Tests for ip/* objects
 tests_testIpAddress_SOURCES= \
        tests/testAddress.cc \

diff --git a/src/icmp/net_db.cc b/src/icmp/net_db.cc
index c9b9178f100707c49c2496581f917a725318de93..f473e33db8a44659a39500612776b5a252b09680 100644 (file)
--- a/src/icmp/net_db.cc
+++ b/src/icmp/net_db.cc
@@ -215,7 +215,7 @@ netdbRelease(netdbEntry * n)
 
     if (n->link_count == 0) {
         netdbHashDelete(n->network);
-        memFree(n, MEM_NETDBENTRY);
+        delete n;
     }
 }
 
@@ -242,11 +242,11 @@ netdbPurgeLRU(void)
     int k = 0;
     int list_count = 0;
     int removed = 0;
-    list = (netdbEntry **)xcalloc(memInUse(MEM_NETDBENTRY), sizeof(netdbEntry *));
+    list = (netdbEntry **)xcalloc(netdbEntry::UseCount(), sizeof(netdbEntry *));
     hash_first(addr_table);
 
     while ((n = (netdbEntry *) hash_next(addr_table))) {
-        assert(list_count < memInUse(MEM_NETDBENTRY));
+        assert(list_count < netdbEntry::UseCount());
         *(list + list_count) = n;
         ++list_count;
     }
@@ -257,7 +257,7 @@ netdbPurgeLRU(void)
           netdbLRU);
 
     for (k = 0; k < list_count; ++k) {
-        if (memInUse(MEM_NETDBENTRY) < Config.Netdb.low)
+        if (netdbEntry::UseCount() < Config.Netdb.low)
             break;
 
         netdbRelease(*(list + k));
@@ -284,11 +284,11 @@ netdbAdd(Ip::Address &addr)
 {
     netdbEntry *n;
 
-    if (memInUse(MEM_NETDBENTRY) > Config.Netdb.high)
+    if (netdbEntry::UseCount() > Config.Netdb.high)
         netdbPurgeLRU();
 
     if ((n = netdbLookupAddr(addr)) == NULL) {
-        n = (netdbEntry *)memAllocate(MEM_NETDBENTRY);
+        n = new netdbEntry;
         netdbHashInsert(n, addr);
     }
 
@@ -642,7 +642,7 @@ netdbReloadState(void)
 
         N.last_use_time = (time_t) atoi(q);
 
-        n = (netdbEntry *)memAllocate(MEM_NETDBENTRY);
+        n = new netdbEntry;
 
         memcpy(n, &N, sizeof(netdbEntry));
 
@@ -683,7 +683,7 @@ netdbFreeNetdbEntry(void *data)
 {
     netdbEntry *n = (netdbEntry *)data;
     safe_free(n->peers);
-    memFree(n, MEM_NETDBENTRY);
+    delete n;
 }
 
 static void
@@ -888,12 +888,6 @@ netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData)
     }
 }
 
-static void
-netdbRegisterWithCacheManager(void)
-{
-    Mgr::RegisterAction("netdb", "Network Measurement Database", netdbDump, 0, 1);
-}
-
 #endif /* USE_ICMP */
 
 /* PUBLIC FUNCTIONS */
@@ -902,14 +896,12 @@ void
 netdbInit(void)
 {
 #if USE_ICMP
-    int n;
-
-    netdbRegisterWithCacheManager();
+    Mgr::RegisterAction("netdb", "Network Measurement Database", netdbDump, 0, 1);
 
     if (addr_table)
         return;
 
-    n = hashPrime(Config.Netdb.high / 4);
+    int n = hashPrime(Config.Netdb.high / 4);
 
     addr_table = hash_create((HASHCMP *) strcmp, n, hash_string);
 
@@ -1003,7 +995,7 @@ netdbDump(StoreEntry * sentry)
                       "RTT",
                       "Hops",
                       "Hostnames");
-    list = (netdbEntry **)xcalloc(memInUse(MEM_NETDBENTRY), sizeof(netdbEntry *));
+    list = (netdbEntry **)xcalloc(netdbEntry::UseCount(), sizeof(netdbEntry *));
     i = 0;
     hash_first(addr_table);
 
@@ -1012,9 +1004,9 @@ netdbDump(StoreEntry * sentry)
         ++i;
     }
 
-    if (i != memInUse(MEM_NETDBENTRY))
+    if (i != netdbEntry::UseCount())
         debugs(38, DBG_CRITICAL, "WARNING: netdb_addrs count off, found " << i <<
-               ", expected " << memInUse(MEM_NETDBENTRY));
+               ", expected " << netdbEntry::UseCount());
 
     qsort((char *) list,
           i,

diff --git a/src/icmp/net_db.h b/src/icmp/net_db.h
index 0ee85789ae1f1e90abf96dc1e283b4f0b6916a64..c560f5481d27c31cfec65d2374009cc7b4e1820f 100644 (file)
--- a/src/icmp/net_db.h
+++ b/src/icmp/net_db.h
@@ -42,23 +42,26 @@ public:
     time_t expires;
 };
 
-// POD
 class netdbEntry
 {
+    MEMPROXY_CLASS(netdbEntry);
+
 public:
+    netdbEntry() { *network = 0; }
+
     hash_link hash;     /* must be first */
     char network[MAX_IPSTRLEN];
-    int pings_sent;
-    int pings_recv;
-    double hops;
-    double rtt;
-    time_t next_ping_time;
-    time_t last_use_time;
-    int link_count;
-    net_db_name *hosts;
-    net_db_peer *peers;
-    int n_peers_alloc;
-    int n_peers;
+    int pings_sent = 0;
+    int pings_recv = 0;
+    double hops = 0;
+    double rtt = 1.0;
+    time_t next_ping_time = 0;
+    time_t last_use_time = 0;
+    int link_count = 0;
+    net_db_name *hosts = nullptr;
+    net_db_peer *peers = nullptr;
+    int n_peers_alloc = 0;
+    int n_peers = 0;
 };
 
 void netdbInit(void);

diff --git a/src/ip/Address.h b/src/ip/Address.h
index 355ab0a71bc9b59a1644cf6cd87d5a1259a24fdd..d8ff6bcfa8aaec764f83e471c51256d973c35fef 100644 (file)
--- a/src/ip/Address.h
+++ b/src/ip/Address.h
@@ -11,6 +11,8 @@
 #ifndef _SQUID_SRC_IP_ADDRESS_H
 #define _SQUID_SRC_IP_ADDRESS_H
 
+#include "ip/forward.h"
+
 #include <iosfwd>
 #include <ostream>
 #if HAVE_SYS_SOCKET_H
@@ -32,10 +34,6 @@
 namespace Ip
 {
 
-/// Length of buffer that needs to be allocated to old a null-terminated IP-string
-// Yuck. But there are still structures that need it to be an 'integer constant'.
-#define MAX_IPSTRLEN  75
-
 /**
  * Holds and manipulates IPv4, IPv6, and Socket Addresses.
  */

diff --git a/src/ip/forward.h b/src/ip/forward.h
index 69a150377ff67cc92af3a677ddf38374f3b536f8..fac21600cf3b4546c07028337b861073c2e78b58 100644 (file)
--- a/src/ip/forward.h
+++ b/src/ip/forward.h
@@ -18,6 +18,10 @@ namespace Ip
 class Address;
 }
 
+/// Length of buffer that needs to be allocated to old a null-terminated IP-string
+// Yuck. But there are still structures that need it to be an 'integer constant'.
+#define MAX_IPSTRLEN  75
+
 typedef uint32_t nfmark_t;
 typedef unsigned char tos_t;
 

diff --git a/src/mem/forward.h b/src/mem/forward.h
index baeab27b60cf28fb839c4cfec3a75a42e4298e84..dd0d3af716dba57a7bd4cec07e813ce07716fed7 100644 (file)
--- a/src/mem/forward.h
+++ b/src/mem/forward.h
@@ -50,7 +50,6 @@ typedef enum {
     MEM_DREAD_CTRL,
     MEM_DWRITE_Q,
     MEM_MD5_DIGEST,
-    MEM_NETDBENTRY,
     MEM_MAX
 } mem_type;
 

diff --git a/src/mem/old_api.cc b/src/mem/old_api.cc
index 85daace68da9169ad33814fc6047d96878bd1970..32e8d6f6e03a94b5f4fd162d019a61ce758f2313 100644 (file)
--- a/src/mem/old_api.cc
+++ b/src/mem/old_api.cc
@@ -448,7 +448,6 @@ Mem::Init(void)
     memDataInit(MEM_LINK_LIST, "link_list", sizeof(link_list), 10);
     memDataInit(MEM_DREAD_CTRL, "dread_ctrl", sizeof(dread_ctrl), 0);
     memDataInit(MEM_DWRITE_Q, "dwrite_q", sizeof(dwrite_q), 0);
-    memDataInit(MEM_NETDBENTRY, "netdbEntry", sizeof(netdbEntry), 0);
     memDataInit(MEM_MD5_DIGEST, "MD5 digest", SQUID_MD5_DIGEST_LENGTH, 0);
     GetPool(MEM_MD5_DIGEST)->setChunkSize(512 * 1024);
 

diff --git a/src/tests/testNetDb.cc b/src/tests/testNetDb.cc
new file mode 100644 (file)
index 0000000..8c18236
--- /dev/null
+++ b/src/tests/testNetDb.cc
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#include "squid.h"
+#include "icmp/net_db.h"
+#include "tests/testNetDb.h"
+#include "unitTestMain.h"
+
+#include <stdexcept>
+
+CPPUNIT_TEST_SUITE_REGISTRATION( testNetDb );
+
+void
+testNetDb::testConstruct()
+{
+    // default construct and destruct
+    {
+        netdbEntry T;
+        CPPUNIT_ASSERT_EQUAL(T.network[0], '\0');
+        CPPUNIT_ASSERT_EQUAL(0, T.pings_sent);
+        CPPUNIT_ASSERT_EQUAL(0, T.pings_recv);
+        CPPUNIT_ASSERT_EQUAL(0.0, T.hops);
+        CPPUNIT_ASSERT_EQUAL(1.0, T.rtt);
+        CPPUNIT_ASSERT_EQUAL(static_cast<time_t>(0), T.next_ping_time);
+        CPPUNIT_ASSERT_EQUAL(static_cast<time_t>(0), T.last_use_time);
+        CPPUNIT_ASSERT_EQUAL(0, T.link_count);
+        CPPUNIT_ASSERT_EQUAL(static_cast<net_db_name*>(nullptr), T.hosts);
+        CPPUNIT_ASSERT_EQUAL(static_cast<net_db_peer*>(nullptr), T.peers);
+        CPPUNIT_ASSERT_EQUAL(0, T.n_peers_alloc);
+        CPPUNIT_ASSERT_EQUAL(0, T.n_peers);
+    }
+
+    // new and delete operations
+    {
+        netdbEntry *T = new netdbEntry;
+        CPPUNIT_ASSERT_EQUAL(T->network[0], '\0');
+        CPPUNIT_ASSERT_EQUAL(0, T->pings_sent);
+        CPPUNIT_ASSERT_EQUAL(0, T->pings_recv);
+        CPPUNIT_ASSERT_EQUAL(0.0, T->hops);
+        CPPUNIT_ASSERT_EQUAL(1.0, T->rtt);
+        CPPUNIT_ASSERT_EQUAL(static_cast<time_t>(0), T->next_ping_time);
+        CPPUNIT_ASSERT_EQUAL(static_cast<time_t>(0), T->last_use_time);
+        CPPUNIT_ASSERT_EQUAL(0, T->link_count);
+        CPPUNIT_ASSERT_EQUAL(static_cast<net_db_name*>(nullptr), T->hosts);
+        CPPUNIT_ASSERT_EQUAL(static_cast<net_db_peer*>(nullptr), T->peers);
+        CPPUNIT_ASSERT_EQUAL(0, T->n_peers_alloc);
+        CPPUNIT_ASSERT_EQUAL(0, T->n_peers);
+        delete T;
+    }
+}
+

diff --git a/src/tests/testNetDb.h b/src/tests/testNetDb.h
new file mode 100644 (file)
index 0000000..294812a
--- /dev/null
+++ b/src/tests/testNetDb.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#ifndef SQUID_SRC_TESTS_TESTNETDB_H
+#define SQUID_SRC_TESTS_TESTNETDB_H
+
+#include <cppunit/extensions/HelperMacros.h>
+
+class testNetDb : public CPPUNIT_NS::TestFixture
+{
+    CPPUNIT_TEST_SUITE( testNetDb );
+    /* note the statement here and then the actual prototype below */
+    CPPUNIT_TEST( testConstruct );
+    CPPUNIT_TEST_SUITE_END();
+
+public:
+
+protected:
+    void testConstruct();
+};
+
+#endif /* SQUID_SRC_TESTS_TESTNETDB_H */
+