------------------------------------------------------------ revno: 13143 revision-id: squid3@treenet.co.nz-20131123005842-ak28deehpjkhwbjm parent: squidadm@squid-cache.org-20131121221203-806g8zb5yxx9f80n committer: Amos Jeffries branch nick: trunk timestamp: Sat 2013-11-23 13:58:42 +1300 message: 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. ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20131123005842-ak28deehpjkhwbjm # target_branch: http://bzr.squid-cache.org/bzr/squid3/trunk/ # testament_sha1: 1b60719439f1b8365d7c2d6c8de852d57dea0dc3 # timestamp: 2013-11-23 01:38:13 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/trunk/ # base_revision_id: squidadm@squid-cache.org-20131121221203-\ # 806g8zb5yxx9f80n # # Begin patch === modified file 'src/HttpHdrCc.h' --- src/HttpHdrCc.h 2013-03-26 10:33:10 +0000 +++ src/HttpHdrCc.h 2013-11-23 00:58:42 +0000 @@ -78,7 +78,7 @@ 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 @@ 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); } === modified file 'src/HttpHeaderTools.cc' --- src/HttpHeaderTools.cc 2013-10-25 00:13:46 +0000 +++ src/HttpHeaderTools.cc 2013-11-23 00:58:42 +0000 @@ -294,7 +294,7 @@ return 0; } /* Make sure it's defined even if empty "" */ - if (!val->defined()) + if (!val->termedBuf()) val->limitInit("", 0); return 1; } === modified file 'src/SquidString.h' --- src/SquidString.h 2013-10-04 13:55:21 +0000 +++ src/SquidString.h 2013-11-23 00:58:42 +0000 @@ -72,14 +72,6 @@ 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 @@ 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! */ === modified file 'src/String.cci' --- src/String.cci 2012-09-22 01:28:35 +0000 +++ src/String.cci 2013-11-23 00:58:42 +0000 @@ -53,16 +53,6 @@ return len_; } -bool String::defined() const -{ - return buf_!=NULL; -} - -bool String::undefined() const -{ - return buf_==NULL; -} - char const * String::rawBuf() const { === modified file 'src/acl/HttpHeaderData.cc' --- src/acl/HttpHeaderData.cc 2013-10-31 19:13:17 +0000 +++ src/acl/HttpHeaderData.cc 2013-11-23 00:58:42 +0000 @@ -75,13 +75,8 @@ 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 * === modified file 'src/adaptation/ecap/MessageRep.cc' --- src/adaptation/ecap/MessageRep.cc 2013-10-25 00:13:46 +0000 +++ src/adaptation/ecap/MessageRep.cc 2013-11-23 00:58:42 +0000 @@ -39,7 +39,7 @@ 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(); } === modified file 'src/adaptation/ecap/XactionRep.cc' --- src/adaptation/ecap/XactionRep.cc 2013-10-25 00:13:46 +0000 +++ src/adaptation/ecap/XactionRep.cc 2013-11-23 00:58:42 +0000 @@ -143,7 +143,7 @@ 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()); } === modified file 'src/adaptation/icap/ModXact.cc' --- src/adaptation/icap/ModXact.cc 2013-11-11 12:09:44 +0000 +++ src/adaptation/icap/ModXact.cc 2013-11-23 00:58:42 +0000 @@ -1352,7 +1352,7 @@ 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 @@ 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); } === modified file 'src/client_side.cc' --- src/client_side.cc 2013-11-11 12:09:44 +0000 +++ src/client_side.cc 2013-11-23 00:58:42 +0000 @@ -3789,7 +3789,7 @@ 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 @@ 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 @@ //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. === modified file 'src/errorpage.cc' --- src/errorpage.cc 2013-10-25 00:13:46 +0000 +++ src/errorpage.cc 2013-11-23 00:58:42 +0000 @@ -839,7 +839,7 @@ 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; === modified file 'src/http.cc' --- src/http.cc 2013-11-18 15:55:05 +0000 +++ src/http.cc 2013-11-23 00:58:42 +0000 @@ -362,7 +362,7 @@ } // 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 @@ // 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 @@ // 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())); } === modified file 'src/redirect.cc' --- src/redirect.cc 2013-10-26 15:56:57 +0000 +++ src/redirect.cc 2013-11-23 00:58:42 +0000 @@ -259,8 +259,7 @@ } #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")); } === modified file 'src/ssl/ErrorDetail.cc' --- src/ssl/ErrorDetail.cc 2013-11-07 10:46:14 +0000 +++ src/ssl/ErrorDetail.cc 2013-11-23 00:58:42 +0000 @@ -404,7 +404,7 @@ 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_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 @@ const String &Ssl::ErrorDetail::toString() const { - if (!errDetailStr.defined()) + if (errDetailStr.size() == 0) buildDetail(); return errDetailStr; } === modified file 'src/ssl/ErrorDetailManager.cc' --- src/ssl/ErrorDetailManager.cc 2013-11-07 10:46:14 +0000 +++ src/ssl/ErrorDetailManager.cc 2013-11-23 00:58:42 +0000 @@ -230,12 +230,11 @@ 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; === modified file 'src/stat.cc' --- src/stat.cc 2013-10-25 00:13:46 +0000 +++ src/stat.cc 2013-11-23 00:58:42 +0000 @@ -2050,7 +2050,7 @@ 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(); } === modified file 'src/store.cc' --- src/store.cc 2013-10-25 00:13:46 +0000 +++ src/store.cc 2013-11-23 00:58:42 +0000 @@ -788,7 +788,7 @@ #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();