From: Amos Jeffries <squid3@treenet.co.nz>
Date: Sat, 23 Nov 2013 00:58:42 +0000 (+1300)
Subject: Remove many uses of String::defined()
X-Git-Tag: SQUID_3_5_0_1~502
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b38b26cb149d3abb3418e0acef8fca4645d01af0;p=thirdparty%2Fsquid.git

Remove many uses of String::defined()

Making it private to String:: so that we can easier avoid dealing with
NULL buffer vs "" problems in future.
---

diff --git a/src/HttpHdrCc.h b/src/HttpHdrCc.h
index af1cb4a656..7a3904dd15 100644
--- a/src/HttpHdrCc.h
+++ b/src/HttpHdrCc.h
@@ -78,7 +78,7 @@ public:
     void Private(String &v) {
         setMask(CC_PRIVATE,true);
         // uses append for multi-line headers
-        if (private_.defined())
+        if (private_.size() > 0)
             private_.append(",");
         private_.append(v);
     }
@@ -90,7 +90,7 @@ public:
     void noCache(String &v) {
         setMask(CC_NO_CACHE,true);
         // uses append for multi-line headers
-        if (no_cache.defined())
+        if (no_cache.size() > 0)
             no_cache.append(",");
         no_cache.append(v);
     }
diff --git a/src/HttpHeaderTools.cc b/src/HttpHeaderTools.cc
index 6a16f03b26..0f6a264607 100644
--- a/src/HttpHeaderTools.cc
+++ b/src/HttpHeaderTools.cc
@@ -294,7 +294,7 @@ httpHeaderParseQuotedString(const char *start, const int len, String *val)
         return 0;
     }
     /* Make sure it's defined even if empty "" */
-    if (!val->defined())
+    if (!val->termedBuf())
         val->limitInit("", 0);
     return 1;
 }
diff --git a/src/SquidString.h b/src/SquidString.h
index 7be3f7170d..dabd1e204f 100644
--- a/src/SquidString.h
+++ b/src/SquidString.h
@@ -71,14 +71,6 @@ public:
     /// throws when size() > MAXINT
     int psize() const;
 
-    /**
-     * \retval true the String has some contents
-     */
-    _SQUID_INLINE_ bool defined() const;
-    /**
-     * \retval true the String does not hold any contents
-     */
-    _SQUID_INLINE_ bool undefined() const;
     /**
      * Returns a raw pointer to the underlying backing store. The caller has been
      * verified not to make any assumptions about null-termination
@@ -121,6 +113,9 @@ private:
     void allocBuffer(size_type sz);
     void setBuffer(char *buf, size_type sz);
 
+    bool defined() const {return buf_!=NULL;}
+    bool undefined() const {return !defined();}
+
     _SQUID_INLINE_ bool nilCmp(bool, bool, int &) const;
 
     /* never reference these directly! */
diff --git a/src/String.cci b/src/String.cci
index 131f1b3391..31107015dd 100644
--- a/src/String.cci
+++ b/src/String.cci
@@ -53,16 +53,6 @@ String::size() const
     return len_;
 }
 
-bool String::defined() const
-{
-    return buf_!=NULL;
-}
-
-bool String::undefined() const
-{
-    return buf_==NULL;
-}
-
 char const *
 String::rawBuf() const
 {
diff --git a/src/acl/HttpHeaderData.cc b/src/acl/HttpHeaderData.cc
index 580ade10ff..ccd7372c93 100644
--- a/src/acl/HttpHeaderData.cc
+++ b/src/acl/HttpHeaderData.cc
@@ -75,13 +75,8 @@ ACLHTTPHeaderData::match(HttpHeader* hdr)
             return false;
     }
 
-    // By now, we know the header is present, but:
-    // HttpHeader::get*() return an undefined String for empty header values;
-    // String::termedBuf() returns NULL for undefined Strings; and
-    // ACLRegexData::match() always fails on NULL strings.
-    // This makes it possible to detect an empty header value using regex:
-    const char *cvalue = value.defined() ? value.termedBuf() : "";
-    return regex_rule->match(cvalue);
+    const SBuf cvalue(value);
+    return regex_rule->match(cvalue.c_str());
 }
 
 wordlist *
diff --git a/src/adaptation/ecap/MessageRep.cc b/src/adaptation/ecap/MessageRep.cc
index 89512fc6e9..ed6cddd6dc 100644
--- a/src/adaptation/ecap/MessageRep.cc
+++ b/src/adaptation/ecap/MessageRep.cc
@@ -39,7 +39,7 @@ Adaptation::Ecap::HeaderRep::value(const Name &name) const
     const String value = squidId == HDR_OTHER ?
                          theHeader.getByName(name.image().c_str()) :
                          theHeader.getStrOrList(squidId);
-    return value.defined() ?
+    return value.size() > 0 ?
            Value::FromTempString(value.termedBuf()) : Value();
 }
 
diff --git a/src/adaptation/ecap/XactionRep.cc b/src/adaptation/ecap/XactionRep.cc
index 7ee3f1d9e3..87aa179961 100644
--- a/src/adaptation/ecap/XactionRep.cc
+++ b/src/adaptation/ecap/XactionRep.cc
@@ -143,7 +143,7 @@ Adaptation::Ecap::XactionRep::usernameValue() const
     if (request->auth_user_request != NULL) {
         if (char const *name = request->auth_user_request->username())
             return libecap::Area::FromTempBuffer(name, strlen(name));
-        else if (request->extacl_user.defined() && request->extacl_user.size())
+        else if (request->extacl_user.size() > 0)
             return libecap::Area::FromTempBuffer(request->extacl_user.rawBuf(),
                                                  request->extacl_user.size());
     }
diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc
index 20a85b5f2c..74938c4df2 100644
--- a/src/adaptation/icap/ModXact.cc
+++ b/src/adaptation/icap/ModXact.cc
@@ -1352,7 +1352,7 @@ void Adaptation::Icap::ModXact::makeRequestHeaders(MemBuf &buf)
     if (virgin.header->header.has(HDR_PROXY_AUTHORIZATION)) {
         String vh=virgin.header->header.getByName("Proxy-Authorization");
         buf.Printf("Proxy-Authorization: " SQUIDSTRINGPH "\r\n", SQUIDSTRINGPRINT(vh));
-    } else if (request->extacl_user.defined() && request->extacl_user.size() && request->extacl_passwd.defined() && request->extacl_passwd.size()) {
+    } else if (request->extacl_user.size() > 0 && request->extacl_passwd.size() > 0) {
         char loginbuf[256];
         snprintf(loginbuf, sizeof(loginbuf), SQUIDSTRINGPH ":" SQUIDSTRINGPH,
                  SQUIDSTRINGPRINT(request->extacl_user),
@@ -1509,7 +1509,7 @@ void Adaptation::Icap::ModXact::makeUsernameHeader(const HttpRequest *request, M
             const char *value = TheConfig.client_username_encode ? old_base64_encode(name) : name;
             buf.Printf("%s: %s\r\n", TheConfig.client_username_header, value);
         }
-    } else if (request->extacl_user.defined() && request->extacl_user.size()) {
+    } else if (request->extacl_user.size() > 0) {
         const char *value = TheConfig.client_username_encode ? old_base64_encode(request->extacl_user.termedBuf()) : request->extacl_user.termedBuf();
         buf.Printf("%s: %s\r\n", TheConfig.client_username_header, value);
     }
diff --git a/src/client_side.cc b/src/client_side.cc
index 7edf6c199e..5347b7f92f 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3789,7 +3789,7 @@ ConnStateData::sslCrtdHandleReply(const HelperReply &reply)
 
 void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &certProperties)
 {
-    certProperties.commonName =  sslCommonName.defined() ? sslCommonName.termedBuf() : sslConnectHostOrIp.termedBuf();
+    certProperties.commonName =  sslCommonName.size() > 0 ? sslCommonName.termedBuf() : sslConnectHostOrIp.termedBuf();
 
     // fake certificate adaptation requires bump-server-first mode
     if (!sslServerBump) {
@@ -3884,7 +3884,7 @@ ConnStateData::getSslContextStart()
         Ssl::CertificateProperties certProperties;
         buildSslCertGenerationParams(certProperties);
         sslBumpCertKey = certProperties.dbKey().c_str();
-        assert(sslBumpCertKey.defined() && sslBumpCertKey[0] != '\0');
+        assert(sslBumpCertKey.size() > 0 && sslBumpCertKey[0] != '\0');
 
         debugs(33, 5, HERE << "Finding SSL certificate for " << sslBumpCertKey << " in cache");
         Ssl::LocalContextStorage & ssl_ctx_cache(Ssl::TheGlobalContextStorage.getLocalStorage(port->s));
@@ -3951,7 +3951,7 @@ ConnStateData::getSslContextDone(SSL_CTX * sslContext, bool isNew)
         //else it is self-signed or untrusted do not attrach any certificate
 
         Ssl::LocalContextStorage & ssl_ctx_cache(Ssl::TheGlobalContextStorage.getLocalStorage(port->s));
-        assert(sslBumpCertKey.defined() && sslBumpCertKey[0] != '\0');
+        assert(sslBumpCertKey.size() > 0 && sslBumpCertKey[0] != '\0');
         if (sslContext) {
             if (!ssl_ctx_cache.add(sslBumpCertKey.termedBuf(), new Ssl::SSL_CTX_Pointer(sslContext))) {
                 // If it is not in storage delete after using. Else storage deleted it.
diff --git a/src/errorpage.cc b/src/errorpage.cc
index 65b92ac247..f268ce3981 100644
--- a/src/errorpage.cc
+++ b/src/errorpage.cc
@@ -839,7 +839,7 @@ ErrorState::Convert(char token, bool building_deny_info_url, bool allowRecursion
         else if (detail) {
             detail->useRequest(request);
             const String &errDetail = detail->toString();
-            if (errDetail.defined()) {
+            if (errDetail.size() > 0) {
                 MemBuf *detail_mb  = ConvertText(errDetail.termedBuf(), false);
                 mb.append(detail_mb->content(), detail_mb->contentSize());
                 delete detail_mb;
diff --git a/src/http.cc b/src/http.cc
index accb570c8b..496001d7bd 100644
--- a/src/http.cc
+++ b/src/http.cc
@@ -362,7 +362,7 @@ HttpStateData::cacheableReply()
         }
 
         // NP: request CC:no-cache only means cache READ is forbidden. STORE is permitted.
-        if (rep->cache_control && rep->cache_control->hasNoCache() && rep->cache_control->noCache().defined()) {
+        if (rep->cache_control && rep->cache_control->hasNoCache() && rep->cache_control->noCache().size() > 0) {
             /* TODO: we are allowed to cache when no-cache= has parameters.
              * Provided we strip away any of the listed headers unless they are revalidated
              * successfully (ie, must revalidate AND these headers are prohibited on stale replies).
@@ -427,7 +427,7 @@ HttpStateData::cacheableReply()
             // HTTPbis WG verdict on this is that it is omitted from the spec due to being 'unexpected' by
             // some. The caching+revalidate is not exactly unsafe though with Squids interpretation of no-cache
             // (without parameters) as equivalent to must-revalidate in the reply.
-        } else if (rep->cache_control->hasNoCache() && !rep->cache_control->noCache().defined() && !REFRESH_OVERRIDE(ignore_must_revalidate)) {
+        } else if (rep->cache_control->hasNoCache() && rep->cache_control->noCache().size() > 0 && !REFRESH_OVERRIDE(ignore_must_revalidate)) {
             debugs(22, 3, HERE << "Authenticated but server reply Cache-Control:no-cache (equivalent to must-revalidate)");
             mayStore = true;
 #endif
@@ -1706,7 +1706,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request,
 
     // Add our own If-None-Match field if the cached entry has a strong ETag.
     // copyOneHeaderFromClientsideRequestToUpstreamRequest() adds client ones.
-    if (request->etag.defined()) {
+    if (request->etag.size() > 0) {
         hdr_out->addEntry(new HttpHeaderEntry(HDR_IF_NONE_MATCH, NULL,
                                               request->etag.termedBuf()));
     }
diff --git a/src/redirect.cc b/src/redirect.cc
index 9a659b0105..4131d1c3ad 100644
--- a/src/redirect.cc
+++ b/src/redirect.cc
@@ -259,8 +259,7 @@ constructHelperQuery(const char *name, helper *hlp, HLPCB *replyHandler, ClientH
     }
 #endif
 
-    // HttpRequest initializes with null_string. So we must check both defined() and size()
-    if (!r->client_ident && http->request->extacl_user.defined() && http->request->extacl_user.size()) {
+    if (!r->client_ident && http->request->extacl_user.size() > 0) {
         r->client_ident = http->request->extacl_user.termedBuf();
         debugs(61, 5, HERE << "acl-user=" << (r->client_ident?r->client_ident:"NULL"));
     }
diff --git a/src/ssl/ErrorDetail.cc b/src/ssl/ErrorDetail.cc
index d599cc5ce4..7fb754f68f 100644
--- a/src/ssl/ErrorDetail.cc
+++ b/src/ssl/ErrorDetail.cc
@@ -404,7 +404,7 @@ static int copy_cn(void *check_data,  ASN1_STRING *cn_data)
     String *str = (String *)check_data;
     if (!str) // no data? abort
         return 0;
-    if (str->defined())
+    if (str->size() > 0)
         str->append(", ");
     str->append((const char *)cn_data->data, cn_data->length);
     return 1;
@@ -501,7 +501,7 @@ const char *Ssl::ErrorDetail::err_descr() const
 
 const char *Ssl::ErrorDetail::err_lib_error() const
 {
-    if (errReason.defined())
+    if (errReason.size() > 0)
         return errReason.termedBuf();
     else if (lib_error_no != SSL_ERROR_NONE)
         return ERR_error_string(lib_error_no, NULL);
@@ -574,7 +574,7 @@ void Ssl::ErrorDetail::buildDetail() const
 
 const String &Ssl::ErrorDetail::toString() const
 {
-    if (!errDetailStr.defined())
+    if (errDetailStr.size() == 0)
         buildDetail();
     return errDetailStr;
 }
diff --git a/src/ssl/ErrorDetailManager.cc b/src/ssl/ErrorDetailManager.cc
index 3109c39166..e136dfa978 100644
--- a/src/ssl/ErrorDetailManager.cc
+++ b/src/ssl/ErrorDetailManager.cc
@@ -230,12 +230,11 @@ Ssl::ErrorDetailFile::parse(const char *buffer, int len, bool eof)
                 entry.error_no = ssl_error;
                 entry.name = errorName;
                 String tmp = parser.getByName("detail");
-                httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), &entry.detail);
+                int detailsParseOk = httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), &entry.detail);
                 tmp = parser.getByName("descr");
-                httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), &entry.descr);
-                bool parseOK = entry.descr.defined() && entry.detail.defined();
+                int descrParseOk = httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), &entry.descr);
 
-                if (!parseOK) {
+                if (!detailsParseOk || !descrParseOk) {
                     debugs(83, DBG_IMPORTANT, HERE <<
                            "WARNING! missing important field for detail error: " <<  errorName);
                     return false;
diff --git a/src/stat.cc b/src/stat.cc
index 38c8a4f33c..94e512cf5a 100644
--- a/src/stat.cc
+++ b/src/stat.cc
@@ -2050,7 +2050,7 @@ statClientRequests(StoreEntry * s)
             p = http->request->auth_user_request->username();
         else
 #endif
-            if (http->request->extacl_user.defined()) {
+            if (http->request->extacl_user.size() > 0) {
                 p = http->request->extacl_user.termedBuf();
             }
 
diff --git a/src/store.cc b/src/store.cc
index b4a9939f6a..4f83302388 100644
--- a/src/store.cc
+++ b/src/store.cc
@@ -788,7 +788,7 @@ StoreEntry::setPublicKey()
 #if X_ACCELERATOR_VARY
             vary = mem_obj->getReply()->header.getList(HDR_X_ACCELERATOR_VARY);
 
-            if (vary.defined()) {
+            if (vary.size() > 0) {
                 /* Again, we own this structure layout */
                 rep->header.putStr(HDR_X_ACCELERATOR_VARY, vary.termedBuf());
                 vary.clean();