From: Amos Jeffries Date: Wed, 19 Oct 2016 21:14:13 +0000 (+1300) Subject: Cleanup: use new/delete allocation from netdbEntry class X-Git-Tag: SQUID_4_0_16~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1a7cfe0289a5c24552850f23ad0a50581a7cbe22;p=thirdparty%2Fsquid.git 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 41f16efd88..4c132c9f70 100644 --- 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 c9b9178f10..f473e33db8 100644 --- 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 0ee85789ae..c560f5481d 100644 --- 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 355ab0a71b..d8ff6bcfa8 100644 --- 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 #include #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 69a150377f..fac21600cf 100644 --- 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 baeab27b60..dd0d3af716 100644 --- 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 85daace68d..32e8d6f6e0 100644 --- 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 index 0000000000..8c182369c4 --- /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 + +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(0), T.next_ping_time); + CPPUNIT_ASSERT_EQUAL(static_cast(0), T.last_use_time); + CPPUNIT_ASSERT_EQUAL(0, T.link_count); + CPPUNIT_ASSERT_EQUAL(static_cast(nullptr), T.hosts); + CPPUNIT_ASSERT_EQUAL(static_cast(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(0), T->next_ping_time); + CPPUNIT_ASSERT_EQUAL(static_cast(0), T->last_use_time); + CPPUNIT_ASSERT_EQUAL(0, T->link_count); + CPPUNIT_ASSERT_EQUAL(static_cast(nullptr), T->hosts); + CPPUNIT_ASSERT_EQUAL(static_cast(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 index 0000000000..294812a198 --- /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 + +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 */ +