From: Amos Jeffries <squid3@treenet.co.nz>
Date: Mon, 24 Oct 2016 12:46:35 +0000 (+1300)
Subject: Cleanup: convert AclDenyInfoList and AclNameList to MEMPROXY_CLASS
X-Git-Tag: SQUID_4_0_16~14
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2e00d18590221781180b593c6bab02f0497bf723;p=thirdparty%2Fsquid.git

Cleanup: convert AclDenyInfoList and AclNameList to MEMPROXY_CLASS

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

Remove now unused MEM_ACL_DENY_INFO_LIST and MEM_ACL_NAME_LIST
memory pools.

Remove apparently unused DenyInfoList global.

Fixes a memory leak on shutdown/reconfigure when
aclDestroyDenyInfoList() frees a list of AclDenyInfoList object
but not the AclNameList objects attached to them.

Fixes a memory leak when reconfigure frees a list of AclDenyInfoList
objects but not the URL/page-name strings attached to them.
---

diff --git a/src/acl/AclDenyInfoList.h b/src/acl/AclDenyInfoList.h
index e071a7d702..c05311e0b0 100644
--- a/src/acl/AclDenyInfoList.h
+++ b/src/acl/AclDenyInfoList.h
@@ -9,18 +9,35 @@
 #ifndef SQUID_ACLDENYINFOLIST_H_
 #define SQUID_ACLDENYINFOLIST_H_
 
+#include "acl/AclNameList.h"
 #include "err_type.h"
-
-class AclNameList;
+#include "errorpage.h"
+#include "mem/forward.h"
 
 /// deny_info representation. Currently a POD.
 class AclDenyInfoList
 {
+    MEMPROXY_CLASS(AclDenyInfoList);
+
 public:
-    err_type err_page_id;
-    char *err_page_name;
-    AclNameList *acl_list;
-    AclDenyInfoList *next;
+    AclDenyInfoList(const char *t) {
+        err_page_name = xstrdup(t);
+        err_page_id = errorReservePageId(t);
+    }
+    ~AclDenyInfoList() {
+        xfree(err_page_name);
+        delete acl_list;
+        while (next) {
+            auto *a = next;
+            next = a->next;
+            a->next = nullptr;
+            delete a;
+        }
+    }
+    err_type err_page_id = ERR_NONE;
+    char *err_page_name = nullptr;
+    AclNameList *acl_list = nullptr;
+    AclDenyInfoList *next = nullptr;
 };
 
 #endif /* SQUID_ACLDENYINFOLIST_H_ */
diff --git a/src/acl/AclNameList.h b/src/acl/AclNameList.h
index ebd625177d..2e514ae569 100644
--- a/src/acl/AclNameList.h
+++ b/src/acl/AclNameList.h
@@ -10,13 +10,24 @@
 #define SQUID_ACL_ACLNAMELIST_H_
 
 #include "acl/forward.h"
+#include "mem/forward.h"
 
-/// list of name-based ACLs. Currently a POD.
+/// list of name-based ACLs
 class AclNameList
 {
+    MEMPROXY_CLASS(AclNameList);
+
 public:
+    AclNameList(const char *t) {
+        xstrncpy(name, t, ACL_NAME_SZ-1);
+    }
+    ~AclNameList() {
+        // recursion is okay, these lists are short
+        delete next;
+    }
+
     char name[ACL_NAME_SZ];
-    AclNameList *next;
+    AclNameList *next = nullptr;
 };
 // TODO: convert to a std::list<string>
 
diff --git a/src/acl/Gadgets.cc b/src/acl/Gadgets.cc
index 017ed9c6a5..409138a19b 100644
--- a/src/acl/Gadgets.cc
+++ b/src/acl/Gadgets.cc
@@ -19,7 +19,6 @@
 #include "squid.h"
 #include "acl/Acl.h"
 #include "acl/AclDenyInfoList.h"
-#include "acl/AclNameList.h"
 #include "acl/Checklist.h"
 #include "acl/Gadgets.h"
 #include "acl/Strategised.h"
@@ -105,9 +104,8 @@ void
 aclParseDenyInfoLine(AclDenyInfoList ** head)
 {
     char *t = NULL;
-    AclDenyInfoList *A = NULL;
-    AclDenyInfoList *B = NULL;
-    AclDenyInfoList **T = NULL;
+    AclDenyInfoList *B;
+    AclDenyInfoList **T;
     AclNameList *L = NULL;
     AclNameList **Tail = NULL;
 
@@ -119,16 +117,13 @@ aclParseDenyInfoLine(AclDenyInfoList ** head)
         return;
     }
 
-    A = (AclDenyInfoList *)memAllocate(MEM_ACL_DENY_INFO_LIST);
-    A->err_page_id = errorReservePageId(t);
-    A->err_page_name = xstrdup(t);
-    A->next = (AclDenyInfoList *) NULL;
+    AclDenyInfoList *A = new AclDenyInfoList(t);
+
     /* next expect a list of ACL names */
     Tail = &A->acl_list;
 
     while ((t = ConfigParser::NextToken())) {
-        L = (AclNameList *)memAllocate(MEM_ACL_NAME_LIST);
-        xstrncpy(L->name, t, ACL_NAME_SZ-1);
+        L = new AclNameList(t);
         *Tail = L;
         Tail = &L->next;
     }
@@ -136,7 +131,7 @@ aclParseDenyInfoLine(AclDenyInfoList ** head)
     if (A->acl_list == NULL) {
         debugs(28, DBG_CRITICAL, "aclParseDenyInfoLine: " << cfg_filename << " line " << config_lineno << ": " << config_input_line);
         debugs(28, DBG_CRITICAL, "aclParseDenyInfoLine: deny_info line contains no ACL's, skipping");
-        memFree(A, MEM_ACL_DENY_INFO_LIST);
+        delete A;
         return;
     }
 
@@ -315,8 +310,7 @@ aclDestroyDenyInfoList(AclDenyInfoList ** list)
         }
 
         a_next = a->next;
-        xfree(a->err_page_name);
-        memFree(a, MEM_ACL_DENY_INFO_LIST);
+        delete a;
     }
 
     *list = NULL;
diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index 0e191a51b8..f9777b6825 100644
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -11,7 +11,6 @@
 #include "squid.h"
 #include "acl/Acl.h"
 #include "acl/AclDenyInfoList.h"
-#include "acl/AclNameList.h"
 #include "acl/AclSizeLimit.h"
 #include "acl/Address.h"
 #include "acl/Gadgets.h"
@@ -2363,12 +2362,10 @@ free_cachemgrpasswd(Mgr::ActionPasswordList ** head)
 static void
 dump_denyinfo(StoreEntry * entry, const char *name, AclDenyInfoList * var)
 {
-    AclNameList *a;
-
     while (var != NULL) {
         storeAppendPrintf(entry, "%s %s", name, var->err_page_name);
 
-        for (a = var->acl_list; a != NULL; a = a->next)
+        for (auto *a = var->acl_list; a != NULL; a = a->next)
             storeAppendPrintf(entry, " %s", a->name);
 
         storeAppendPrintf(entry, "\n");
@@ -2386,24 +2383,8 @@ parse_denyinfo(AclDenyInfoList ** var)
 void
 free_denyinfo(AclDenyInfoList ** list)
 {
-    AclDenyInfoList *a = NULL;
-    AclDenyInfoList *a_next = NULL;
-    AclNameList *l = NULL;
-    AclNameList *l_next = NULL;
-
-    for (a = *list; a; a = a_next) {
-        for (l = a->acl_list; l; l = l_next) {
-            l_next = l->next;
-            memFree(l, MEM_ACL_NAME_LIST);
-            l = NULL;
-        }
-
-        a_next = a->next;
-        memFree(a, MEM_ACL_DENY_INFO_LIST);
-        a = NULL;
-    }
-
-    *list = NULL;
+    delete *list;
+    *list = nullptr;
 }
 
 static void
diff --git a/src/globals.h b/src/globals.h
index 2ab7c0ad39..35176d609f 100644
--- a/src/globals.h
+++ b/src/globals.h
@@ -9,7 +9,6 @@
 #ifndef SQUID_GLOBALS_H
 #define SQUID_GLOBALS_H
 
-#include "acl/AclDenyInfoList.h"
 #include "CacheDigest.h"
 #include "defines.h"
 #include "hash.h"
@@ -62,8 +61,6 @@ extern int DnsSocketB;      /* -1 */
 extern int n_disk_objects;  /* 0 */
 extern IoStats IOStats;
 
-extern AclDenyInfoList *DenyInfoList;   /* NULL */
-
 extern struct timeval squid_start;
 extern int starting_up; /* 1 */
 extern int shutting_down;   /* 0 */
diff --git a/src/mem/forward.h b/src/mem/forward.h
index dd0d3af716..d7adc84629 100644
--- a/src/mem/forward.h
+++ b/src/mem/forward.h
@@ -44,8 +44,6 @@ typedef enum {
     MEM_16K_BUF,
     MEM_32K_BUF,
     MEM_64K_BUF,
-    MEM_ACL_DENY_INFO_LIST,
-    MEM_ACL_NAME_LIST,
     MEM_LINK_LIST,
     MEM_DREAD_CTRL,
     MEM_DWRITE_Q,
diff --git a/src/mem/old_api.cc b/src/mem/old_api.cc
index 32e8d6f6e0..9ce3d444bd 100644
--- a/src/mem/old_api.cc
+++ b/src/mem/old_api.cc
@@ -9,8 +9,6 @@
 /* DEBUG: section 13    High Level Memory Pool Management */
 
 #include "squid.h"
-#include "acl/AclDenyInfoList.h"
-#include "acl/AclNameList.h"
 #include "base/PackableStream.h"
 #include "ClientInfo.h"
 #include "dlink.h"
@@ -442,9 +440,6 @@ Mem::Init(void)
     memDataInit(MEM_16K_BUF, "16K Buffer", 16384, 10, false);
     memDataInit(MEM_32K_BUF, "32K Buffer", 32768, 10, false);
     memDataInit(MEM_64K_BUF, "64K Buffer", 65536, 10, false);
-    memDataInit(MEM_ACL_DENY_INFO_LIST, "AclDenyInfoList",
-                sizeof(AclDenyInfoList), 0);
-    memDataInit(MEM_ACL_NAME_LIST, "acl_name_list", sizeof(AclNameList), 0);
     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);