From: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Date: Fri, 7 Jun 2024 06:43:46 +0000 (+0000)
Subject: Allocate fast-checking ACLFilledChecklists on stack (#1835)
X-Git-Tag: SQUID_7_0_1~107
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ebe87c093363a59d42b7c36bbce733a3eae9a7d6;p=thirdparty%2Fsquid.git

Allocate fast-checking ACLFilledChecklists on stack (#1835)

There is no need for dynamic allocation risks and performance overheads
in these simple "immediate fastCheck()" cases.

This change converts all dynamically allocated checklists that use only
fastCheck() calls except one: Security::PeerConnector::initialize() has
to allocate its checklist dynamically to pass it to SSL_set_ex_data().
---

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
index a2dc68f9dd..9700e7ec1b 100644
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@ -798,11 +798,10 @@ HttpRequest::manager(const CbcPointer<ConnStateData> &aMgr, const AccessLogEntry
         const bool proxyProtocolPort = port ? port->flags.proxySurrogate : false;
         if (flags.interceptTproxy && !proxyProtocolPort) {
             if (Config.accessList.spoof_client_ip) {
-                const auto checklist = new ACLFilledChecklist(Config.accessList.spoof_client_ip, this);
-                checklist->al = al;
-                checklist->syncAle(this, nullptr);
-                flags.spoofClientIp = checklist->fastCheck().allowed();
-                delete checklist;
+                ACLFilledChecklist checklist(Config.accessList.spoof_client_ip, this);
+                checklist.al = al;
+                checklist.syncAle(this, nullptr);
+                flags.spoofClientIp = checklist.fastCheck().allowed();
             } else
                 flags.spoofClientIp = true;
         } else
diff --git a/src/adaptation/icap/Launcher.cc b/src/adaptation/icap/Launcher.cc
index 8eb8c1e938..a7184184d3 100644
--- a/src/adaptation/icap/Launcher.cc
+++ b/src/adaptation/icap/Launcher.cc
@@ -140,12 +140,9 @@ bool Adaptation::Icap::Launcher::canRepeat(Adaptation::Icap::XactAbortInfo &info
     if (info.icapReply->sline.status() == Http::scNone) // failed to parse the reply; I/O err
         return true;
 
-    const auto cl = new ACLFilledChecklist(TheConfig.repeat, info.icapRequest);
-    cl->updateReply(info.icapReply);
-
-    bool result = cl->fastCheck().allowed();
-    delete cl;
-    return result;
+    ACLFilledChecklist cl(TheConfig.repeat, info.icapRequest);
+    cl.updateReply(info.icapReply);
+    return cl.fastCheck().allowed();
 }
 
 /* ICAPXactAbortInfo */
diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index 0e13953425..4754573dd2 100644
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -844,9 +844,10 @@ clientReplyContext::blockedHit() const
         return false; // internal content "hits" cannot be blocked
 
     {
-        std::unique_ptr<ACLFilledChecklist> chl(clientAclChecklistCreate(Config.accessList.sendHit, http));
-        chl->updateReply(&http->storeEntry()->mem().freshestReply());
-        return !chl->fastCheck().allowed(); // when in doubt, block
+        ACLFilledChecklist chl(Config.accessList.sendHit, nullptr);
+        clientAclChecklistFill(chl, http);
+        chl.updateReply(&http->storeEntry()->mem().freshestReply());
+        return !chl.fastCheck().allowed(); // when in doubt, block
     }
 }
 
diff --git a/src/http/Stream.cc b/src/http/Stream.cc
index d5d222049c..17c82e540b 100644
--- a/src/http/Stream.cc
+++ b/src/http/Stream.cc
@@ -290,9 +290,10 @@ Http::Stream::sendStartOfMessage(HttpReply *rep, StoreIOBuffer bodyData)
 #if USE_DELAY_POOLS
     for (const auto &pool: MessageDelayPools::Instance()->pools) {
         if (pool->access) {
-            std::unique_ptr<ACLFilledChecklist> chl(clientAclChecklistCreate(pool->access, http));
-            chl->updateReply(rep);
-            const auto answer = chl->fastCheck();
+            ACLFilledChecklist chl(pool->access, nullptr);
+            clientAclChecklistFill(chl, http);
+            chl.updateReply(rep);
+            const auto answer = chl.fastCheck();
             if (answer.allowed()) {
                 writeQuotaHandler = pool->createBucket();
                 fd_table[clientConnection->fd].writeQuotaHandler = writeQuotaHandler;
diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc
index d23c0b3d53..48fbbf45c1 100644
--- a/src/security/PeerConnector.cc
+++ b/src/security/PeerConnector.cc
@@ -33,6 +33,8 @@
 #include "ssl/cert_validate_message.h"
 #include "ssl/Config.h"
 #include "ssl/helper.h"
+
+#include <optional>
 #endif
 
 Security::PeerConnector::PeerConnector(const Comm::ConnectionPointer &aServerConn, const AsyncCallback<EncryptorAnswer> &aCallback, const AccessLogEntryPointer &alp, const time_t timeout):
@@ -384,11 +386,11 @@ Security::PeerConnector::sslCrtvdCheckForErrors(Ssl::CertValidationResponse cons
 {
     Must(Comm::IsConnOpen(serverConnection()));
 
-    ACLFilledChecklist *check = nullptr;
     Security::SessionPointer session(fd_table[serverConnection()->fd].ssl);
 
+    std::optional<ACLFilledChecklist> check;
     if (acl_access *acl = ::Config.ssl_client.cert_error) {
-        check = new ACLFilledChecklist(acl, request.getRaw());
+        check.emplace(acl, request.getRaw());
         fillChecklist(*check);
     }
 
@@ -427,8 +429,6 @@ Security::PeerConnector::sslCrtvdCheckForErrors(Ssl::CertValidationResponse cons
         else
             errs->push_back_unique(Security::CertError(i->error_no, i->cert, i->error_depth));
     }
-    if (check)
-        delete check;
 
     return errs;
 }