From: Francesco Chemolli Date: Sun, 29 Jan 2023 21:59:41 +0000 (+0000) Subject: Improve error message storage in Dns::LookupDetails (#1241) X-Git-Tag: SQUID_6_0_1~28 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=8651e12628dfec0639a624b676f4f75211e7b9a8;p=thirdparty%2Fsquid.git Improve error message storage in Dns::LookupDetails (#1241) Removed one more deprecated String usage and optimized handling of a common "no error, no lookup" case as well as portions of the "no-error lookup" code paths. Further optimizations need similar ipcache_entry and fqdncache_entry error storage changes (at least). Co-authored-by: Alex Rousskov --- diff --git a/src/dns/LookupDetails.cc b/src/dns/LookupDetails.cc index 01c15abddb..743b6744a9 100644 --- a/src/dns/LookupDetails.cc +++ b/src/dns/LookupDetails.cc @@ -16,8 +16,8 @@ Dns::LookupDetails::print(std::ostream &os) const { if (wait > 0) os << "lookup_wait=" << wait; - if (error.size()) - os << " lookup_err=" << error; + if (error) + os << " lookup_err=" << *error; return os; } diff --git a/src/dns/LookupDetails.h b/src/dns/LookupDetails.h index 1e28dcdcba..fc7c36eb4f 100644 --- a/src/dns/LookupDetails.h +++ b/src/dns/LookupDetails.h @@ -11,7 +11,9 @@ #ifndef SQUID_DNS_LOOKUPDETAILS_H #define SQUID_DNS_LOOKUPDETAILS_H -#include "SquidString.h" +#include "sbuf/SBuf.h" + +#include namespace Dns { @@ -20,13 +22,21 @@ namespace Dns class LookupDetails { public: - LookupDetails() : wait(-1) {} ///< no error, no lookup delay (i.e., no lookup) - LookupDetails(const String &anError, int aWait) : error(anError), wait(aWait) {} + /// no lookup attempt: no error and no lookup delay + LookupDetails(): wait(-1) {} + + /// details a possible lookup attempt + /// \param anError either a failed attempt error message or an empty string + /// \param aWait \copydoc wait + LookupDetails(const SBuf &anError, const int aWait): + error(anError.isEmpty() ? std::nullopt : std::make_optional(anError)), + wait(aWait) + {} std::ostream &print(std::ostream &os) const; public: - String error; ///< error message for unsuccessful lookups; empty otherwise + const std::optional error; ///< error message (if any) int wait; ///< msecs spent waiting for the lookup (if any) or -1 (if none) }; diff --git a/src/errorpage.cc b/src/errorpage.cc index 4d257bc8c6..c4287f4140 100644 --- a/src/errorpage.cc +++ b/src/errorpage.cc @@ -828,8 +828,8 @@ ErrorState::Dump(MemBuf * mb) if (auth_user_request.getRaw() && auth_user_request->denyMessage()) str.appendf("Auth ErrMsg: %s\r\n", auth_user_request->denyMessage()); #endif - if (dnsError.size() > 0) - str.appendf("DNS ErrMsg: %s\r\n", dnsError.termedBuf()); + if (dnsError) + str.appendf("DNS ErrMsg: " SQUIDSBUFPH "\r\n", SQUIDSBUFPRINT(*dnsError)); /* - TimeStamp */ str.appendf("TimeStamp: %s\r\n\r\n", Time::FormatRfc1123(squid_curtime)); @@ -1212,8 +1212,8 @@ ErrorState::compileLegacyCode(Build &build) case 'z': if (building_deny_info_url) break; - if (dnsError.size() > 0) - p = dnsError.termedBuf(); + if (dnsError) + p = dnsError->c_str(); else if (ftp.cwd_msg) p = ftp.cwd_msg; else diff --git a/src/errorpage.h b/src/errorpage.h index d711a25299..1d3b530451 100644 --- a/src/errorpage.h +++ b/src/errorpage.h @@ -24,6 +24,8 @@ /* auth/UserRequest.h is empty unless USE_AUTH is defined */ #include "auth/UserRequest.h" +#include + /// error page callback typedef void ERCB(int fd, void *, size_t); @@ -176,7 +178,7 @@ public: char *url = nullptr; int xerrno = 0; unsigned short port = 0; - String dnsError; ///< DNS lookup error message + std::optional dnsError; ///< DNS lookup error message time_t ttl = 0; Ip::Address src_addr; diff --git a/src/fqdncache.cc b/src/fqdncache.cc index 1f9978bd56..78670fb235 100644 --- a/src/fqdncache.cc +++ b/src/fqdncache.cc @@ -304,7 +304,7 @@ fqdncacheCallback(fqdncache_entry * f, int wait) f->handler = nullptr; if (cbdataReferenceValidDone(f->handlerData, &cbdata)) { - const Dns::LookupDetails details(f->error_message, wait); + const Dns::LookupDetails details(SBuf(f->error_message), wait); callback(f->name_count ? f->names[0] : nullptr, details, cbdata); } @@ -422,7 +422,7 @@ fqdncache_nbgethostbyaddr(const Ip::Address &addr, FQDNH * handler, void *handle if (name[0] == '\0') { debugs(35, 4, "fqdncache_nbgethostbyaddr: Invalid name!"); - const Dns::LookupDetails details("Invalid hostname", -1); // error, no lookup + static const Dns::LookupDetails details(SBuf("Invalid hostname"), -1); // error, no lookup if (handler) handler(nullptr, details, handlerData); return; diff --git a/src/ipcache.cc b/src/ipcache.cc index fbb0a217c0..4a0630b48d 100644 --- a/src/ipcache.cc +++ b/src/ipcache.cc @@ -280,7 +280,7 @@ IpCacheLookupForwarder::forwardLookup(const char *error) // are sequential. Give it just the new, yet-unaccounted-for delay. if (receiverObj.set()) { if (auto receiver = receiverObj.valid()) { - receiver->noteLookup(Dns::LookupDetails(error, additionalLookupDelay())); + receiver->noteLookup(Dns::LookupDetails(SBuf(error), additionalLookupDelay())); lastLookupEnd = current_time; } } @@ -445,7 +445,7 @@ ipcacheCallback(ipcache_entry *i, const bool hit, const int wait) if (hit) i->handler.forwardHits(i->addrs); - const Dns::LookupDetails details(i->error_message, wait); + const Dns::LookupDetails details(SBuf(i->error_message), wait); i->handler.finalCallback(&i->addrs, details); ipcacheUnlockEntry(i); @@ -620,7 +620,7 @@ ipcache_nbgethostbyname_(const char *name, IpCacheLookupForwarder handler) if (name == nullptr || name[0] == '\0') { debugs(14, 4, "ipcache_nbgethostbyname: Invalid name!"); ++IpcacheStats.invalid; - const Dns::LookupDetails details("Invalid hostname", -1); // error, no lookup + static const Dns::LookupDetails details(SBuf("Invalid hostname"), -1); // error, no lookup handler.finalCallback(nullptr, details); return; }