On 20/11/2013 1:19 a.m., Amos Jeffries wrote:
> [ And here is the new patch. ]
>
>
> This patch implements Alex's suggestion for step 1 of String removal by ...
>
> * Replace String class internal buffer with an SBuf.
> * Move converters to/from String and SBuf from SBuf API to String API.
>
>
> This method allows all String code to be converted quickly to SBuf
> without so much worry about correct API conversion elsewhere around the
> code since the wrapper provides exemplar cut-n-paste code to work from.
>
> Addition of a SBuf constructor and toSBuf() accessor to String allows
> code using String to be converted directly to the SBuf API in smaller
> portions later. These obsolete the SBuf API converters to String and
> help reduce the SBuf API.
>
>
> Notes for auditing:
> Please audit this with a sole view of checking that the original
> String:: API semantics and behaviour are unchanged by the internals
> re-write.
> This patch seeks to *avoid* several things which would normally take up
> auditing time:
> * fixing any bugs in the String API semantics or behaviour
> * fixing any bugs in the code using String
> * using correct C++ coding styles or patterns.
>
> These bugs, behaviour and coding usage checks will be done on the
> following transition patches converting code variables from String to
> SBuf type.
>
> Amos
>
To avoid problems with NULL and defined() the callers of termedBuf()
have been given an audit and updated to check for and cope with "" empty
strings. And termedBuf() altered to return c_str() instead of NULL or
c_str().
NP: there seems to be a regression in SBuf::rfind() which is preventing
a fill unit-test run. But usage as a private proxy is not showing
anything amiss.
Amos
=== modified file 'src/Makefile.am'
--- src/Makefile.am 2013-12-17 17:05:17 +0000
+++ src/Makefile.am 2013-12-19 10:45:53 +0000
@@ -23,7 +23,10 @@
SBuf.h \
SBuf.cc \
SBufExceptions.h \
- SBufExceptions.cc
+ SBufExceptions.cc \
+ SquidString.h \
+ String.cc \
+ String.cci
STOREMETA_SOURCE = \
StoreMeta.cc \
@@ -497,7 +500,6 @@
StatCounters.cc \
StatHist.h \
StatHist.cc \
- String.cc \
StrList.h \
StrList.cc \
stmem.cc \
@@ -589,8 +591,6 @@
MemBuf.h \
Store.cci \
StoreEntryStream.h \
- String.cci \
- SquidString.h \
SquidTime.h
BUILT_SOURCES = \
@@ -728,11 +728,11 @@
MemBuf.cci \
MemBuf.h \
Parsing.h \
+ $(SBUF_SOURCE) \
store_key_md5.h \
store_key_md5.cc \
tests/stub_StoreMeta.cc \
StoreMetaUnpacker.cc \
- String.cc \
SquidNew.cc \
tests/stub_time.cc \
ufsdump.cc \
@@ -1156,12 +1156,10 @@
Notes.cc \
Packer.cc \
Packer.h \
- SquidString.h \
SquidTime.h \
$(SBUF_SOURCE) \
SBufDetailedStats.h \
tests/stub_SBufDetailedStats.cc \
- String.cc \
StrList.h \
StrList.cc \
log/access_log.h \
@@ -1268,7 +1266,6 @@
$(SBUF_SOURCE) \
SBufDetailedStats.h \
tests/stub_SBufDetailedStats.cc \
- String.cc \
store_dir.cc \
StoreIOState.cc \
tests/stub_StoreMeta.cc \
@@ -1387,7 +1384,6 @@
HttpRequestMethod.cc \
Mem.h \
tests/stub_mem.cc \
- String.cc \
tests/testCacheManager.cc \
tests/testCacheManager.h \
tests/testMain.cc \
@@ -1703,7 +1699,6 @@
store_swapmeta.cc \
repl_modules.h \
store.cc \
- String.cc \
StrList.h \
StrList.cc \
tests/stub_SwapDir.cc \
@@ -1975,7 +1970,6 @@
tests/stub_StoreMeta.cc \
StoreMetaUnpacker.cc \
StoreSwapLogData.cc \
- String.cc \
tests/stub_SwapDir.cc \
tests/CapturingStoreEntry.h \
tests/testEvent.cc \
@@ -2220,7 +2214,6 @@
tests/stub_StoreMeta.cc \
StoreMetaUnpacker.cc \
StoreSwapLogData.cc \
- String.cc \
StrList.h \
StrList.cc \
tests/stub_SwapDir.cc \
@@ -2461,7 +2454,6 @@
tests/stub_StoreMeta.cc \
StoreMetaUnpacker.cc \
StoreSwapLogData.cc \
- String.cc \
StrList.h \
StrList.cc \
tests/stub_SwapDir.cc \
@@ -2538,7 +2530,6 @@
MemBuf.h \
Mem.h \
tests/stub_mem.cc \
- String.cc \
cache_cf.h \
YesNoNone.h \
$(SBUF_SOURCE) \
@@ -2582,7 +2573,6 @@
HttpRequestMethod.cc \
Mem.h \
tests/stub_mem.cc \
- String.cc \
tests/testHttpRequest.h \
tests/testHttpRequest.cc \
tests/testHttpRequestMethod.h \
@@ -2887,7 +2877,6 @@
$(SBUF_SOURCE) \
SBufDetailedStats.h \
tests/stub_SBufDetailedStats.cc \
- String.cc \
StrList.h \
StrList.cc \
SwapDir.cc \
@@ -2991,7 +2980,6 @@
Mem.h \
tests/stub_mem.cc \
MemBuf.cc \
- String.cc \
$(SBUF_SOURCE) \
SBufDetailedStats.h \
tests/stub_SBufDetailedStats.cc \
@@ -3124,7 +3112,6 @@
$(SBUF_SOURCE) \
SBufDetailedStats.h \
tests/stub_SBufDetailedStats.cc \
- String.cc \
tests/stub_debug.cc \
tests/stub_client_side_request.cc \
tests/stub_http.cc \
@@ -3305,7 +3292,6 @@
$(SBUF_SOURCE) \
SBufDetailedStats.h \
tests/stub_SBufDetailedStats.cc \
- String.cc \
StrList.h \
StrList.cc \
SwapDir.cc \
@@ -3561,7 +3547,6 @@
tests/stub_StoreMeta.cc \
StoreMetaUnpacker.cc \
StoreSwapLogData.cc \
- String.cc \
StrList.h \
StrList.cc \
tests/stub_SwapDir.cc \
@@ -3657,8 +3642,6 @@
tests/stub_store.cc \
tests/stub_store_stats.cc \
tests/stub_tools.cc \
- SquidString.h \
- String.cc \
tests/stub_wordlist.cc \
tests/stub_MemBuf.cc
nodist_tests_testSBuf_SOURCES=$(TESTSOURCES)
@@ -3697,7 +3680,6 @@
tests/stub_store_stats.cc \
tests/stub_tools.cc \
SquidString.h \
- String.cc \
tests/stub_wordlist.cc \
tests/stub_MemBuf.cc
nodist_tests_testSBufList_SOURCES=$(TESTSOURCES)
@@ -3723,7 +3705,6 @@
$(SBUF_SOURCE) \
SBufDetailedStats.h \
tests/stub_SBufDetailedStats.cc \
- String.cc \
ConfigParser.cc \
fatal.h \
tests/stub_fatal.cc \
@@ -3763,7 +3744,6 @@
tests/stub_MemBuf.cc \
StatHist.cc \
StatHist.h \
- String.cc \
tests/stub_cache_manager.cc \
tests/stub_comm.cc \
tests/stub_debug.cc \
@@ -3777,9 +3757,13 @@
tests/stub_pconn.cc \
tests/stub_stmem.cc \
repl_modules.h \
+ $(SBUF_SOURCE) \
+ SBufDetailedStats.h \
+ tests/stub_SBufDetailedStats.cc \
tests/stub_store.cc \
tests/stub_store_stats.cc \
tools.h \
+ tests/stub_time.cc \
tests/stub_tools.cc \
tests/testMain.cc \
tests/testStatHist.cc \
=== modified file 'src/SBuf.cc'
--- src/SBuf.cc 2013-12-18 17:53:43 +0000
+++ src/SBuf.cc 2013-12-19 07:56:42 +0000
@@ -117,16 +117,6 @@
++stats.live;
}
-SBuf::SBuf(const String &S)
- : store_(GetStorePrototype()), off_(0), len_(0)
-{
- debugs(24, 8, id << " created from string");
- assign(S.rawBuf(), S.size());
- ++stats.alloc;
- ++stats.allocFromString;
- ++stats.live;
-}
-
SBuf::SBuf(const std::string &s)
: store_(GetStorePrototype()), off_(0), len_(0)
{
@@ -758,7 +748,7 @@
os <<
"SBuf stats:\nnumber of allocations: " << alloc <<
"\ncopy-allocations: " << allocCopy <<
- "\ncopy-allocations from SquidString: " << allocFromString <<
+ "\ncopy-allocations from std::string: " << allocFromString <<
"\ncopy-allocations from C String: " << allocFromCString <<
"\nlive references: " << live <<
"\nno-copy assignments: " << assignFast <<
@@ -825,15 +815,6 @@
throw OutOfBoundsException(*this, pos, __FILE__, __LINE__);
}
-String
-SBuf::toString() const
-{
- String rv;
- rv.limitInit(buf(), length());
- ++stats.copyOut;
- return rv;
-}
-
/** re-allocate the backing store of the SBuf.
*
* If there are contents in the SBuf, they will be copied over.
=== modified file 'src/SBuf.h'
--- src/SBuf.h 2013-12-18 17:53:43 +0000
+++ src/SBuf.h 2013-12-19 07:56:42 +0000
@@ -32,7 +32,6 @@
#include "base/InstanceId.h"
#include "MemBlob.h"
#include "SBufExceptions.h"
-#include "SquidString.h"
#if HAVE_CLIMITS
#include <climits>
@@ -134,12 +133,6 @@
*/
explicit SBuf(const char *S, size_type n = npos);
- /** Constructor: import SquidString, copying contents.
- *
- * This method will be removed once SquidString has gone.
- */
- explicit SBuf(const String &S);
-
/// Constructor: import std::string. Contents are copied.
explicit SBuf(const std::string &s);
@@ -313,7 +306,7 @@
/** exports a pointer to the SBuf internal storage.
* \warning ACCESSING RAW STORAGE IS DANGEROUS!
*
- * Returns a ead-only pointer to SBuf's content. No terminating null
+ * Returns a read-only pointer to SBuf's content. No terminating null
* character is appended (use c_str() for that).
* The returned value points to an internal location whose contents
* are guaranteed to remain unchanged only until the next call
@@ -551,12 +544,6 @@
*/
SBuf toUpper() const;
- /** String export function
- * converts the SBuf to a legacy String, by copy.
- * \deprecated
- */
- String toString() const;
-
/// std::string export function
std::string toStdString() const { return std::string(buf(),length()); }
=== modified file 'src/Server.cc'
--- src/Server.cc 2013-12-19 00:55:46 +0000
+++ src/Server.cc 2013-12-19 07:56:42 +0000
@@ -478,7 +478,7 @@
absUrl = NULL;
hdrUrl = rep->header.getStr(hdr);
- if (hdrUrl == NULL) {
+ if (!hdrUrl || !*hdrUrl) {
return;
}
=== modified file 'src/SquidString.h'
--- src/SquidString.h 2013-12-20 05:55:43 +0000
+++ src/SquidString.h 2013-12-19 08:06:39 +0000
@@ -33,6 +33,8 @@
#ifndef SQUID_STRING_H
#define SQUID_STRING_H
+#include "SBuf.h"
+
#if HAVE_OSTREAM
#include <ostream>
#endif
@@ -50,15 +52,24 @@
_SQUID_INLINE_ String();
String(char const *);
String(String const &);
- ~String();
-
- typedef size_t size_type; //storage size intentionally unspecified
- const static size_type npos = static_cast<size_type>(-1);
+
+ explicit String(SBuf const &s) : buf_(s), defined_(s.length() > 0) {}
+ SBuf &toSBuf() const {return buf_;}
+
+ typedef SBuf::size_type size_type; //storage size intentionally unspecified
+ const static size_type npos = SBuf::npos;
String &operator =(char const *);
String &operator =(String const &);
+ String &operator =(SBuf const &s) {
+ defined_=(s.length() > 0); // SBuf is always defined; estimating
+ buf_=s;
+ return *this;
+ }
bool operator ==(String const &) const;
+ // {return defined() == s.defined() && buf_ == s.buf_;}
bool operator !=(String const &) const;
+ // {return defined() != s.defined() || buf_ != s.buf_;}
/**
* Retrieve a single character in the string.
@@ -109,25 +120,18 @@
_SQUID_INLINE_ void cut(size_type newLength);
private:
- void allocAndFill(const char *str, int len);
- void allocBuffer(size_type sz);
- void setBuffer(char *buf, size_type sz);
-
- bool defined() const {return buf_!=NULL;}
- bool undefined() const {return !defined();}
+ bool defined() const {return defined_;}
_SQUID_INLINE_ bool nilCmp(bool, bool, int &) const;
- /* never reference these directly! */
- size_type size_; /* buffer size; 64K limit */
-
- size_type len_; /* current length */
-
- char *buf_;
-
- _SQUID_INLINE_ void set(char const *loc, char const ch);
- _SQUID_INLINE_ void cutPointer(char const *loc);
-
+ mutable SBuf buf_;
+
+ // SquidString used to track whether buf_ was NULL or not
+ // and some code makes use of this property.
+ // Until that is all checked and defined()/undefined() erased
+ // we explicitly track whether SquidString would have had
+ // buf_ allocated.
+ mutable bool defined_;
};
_SQUID_INLINE_ std::ostream & operator<<(std::ostream& os, String const
&aString);
=== modified file 'src/StrList.cc'
--- src/StrList.cc 2012-08-29 09:34:34 +0000
+++ src/StrList.cc 2013-12-10 12:34:23 +0000
@@ -113,7 +113,7 @@
if (!*pos) {
*pos = str->termedBuf();
- if (!*pos)
+ if (!*pos || !**pos)
return 0;
}
=== modified file 'src/String.cc'
--- src/String.cc 2012-10-04 09:14:06 +0000
+++ src/String.cc 2013-11-21 10:31:43 +0000
@@ -37,6 +37,7 @@
#include "mgr/Registration.h"
#include "profiler/Profiler.h"
#include "Store.h"
+#include "SquidString.h"
#if HAVE_LIMITS_H
#include <limits.h>
@@ -49,37 +50,10 @@
return size();
}
-// low-level buffer allocation,
-// does not free old buffer and does not adjust or look at len_
-void
-String::allocBuffer(String::size_type sz)
-{
- PROF_start(StringInitBuf);
- assert (undefined());
- char *newBuffer = (char*)memAllocString(sz, &sz);
- setBuffer(newBuffer, sz);
- PROF_stop(StringInitBuf);
-}
-
-// low-level buffer assignment
-// does not free old buffer and does not adjust or look at len_
-void
-String::setBuffer(char *aBuf, String::size_type aSize)
-{
- assert(undefined());
- assert(aSize < 65536);
- buf_ = aBuf;
- size_ = aSize;
-}
-
-String::String(char const *aString) : size_(0), len_(0), buf_(NULL)
-{
- if (aString)
- allocAndFill(aString, strlen(aString));
-#if DEBUGSTRINGS
-
- StringRegistry::Instance().add(this);
-#endif
+String::String(char const *aString) :
+ buf_(aString),
+ defined_(aString != NULL)
+{
}
String &
@@ -92,9 +66,8 @@
String &
String::operator =(String const &old)
{
- clean(); // TODO: optimize to avoid cleaning the buffer we can use
- if (old.size() > 0)
- allocAndFill(old.rawBuf(), old.size());
+ buf_ = old.buf_;
+ defined_ = old.defined_;
return *this;
}
@@ -121,31 +94,11 @@
String::limitInit(const char *str, int len)
{
clean(); // TODO: optimize to avoid cleaning the buffer we can use
- allocAndFill(str, len);
-}
-
-// Allocates the buffer to fit the supplied string and fills it.
-// Does not clean.
-void
-String::allocAndFill(const char *str, int len)
-{
- PROF_start(StringAllocAndFill);
- assert(this && str);
- allocBuffer(len + 1);
- len_ = len;
- memcpy(buf_, str, len);
- buf_[len] = '\0';
- PROF_stop(StringAllocAndFill);
-}
-
-String::String(String const &old) : size_(0), len_(0), buf_(NULL)
-{
- if (old.size() > 0)
- allocAndFill(old.rawBuf(), old.size());
-#if DEBUGSTRINGS
-
- StringRegistry::Instance().add(this);
-#endif
+ append(str, len);
+}
+
+String::String(String const &old) : buf_(old.buf_), defined_(old.defined_)
+{
}
void
@@ -154,34 +107,23 @@
PROF_start(StringClean);
assert(this);
- /* TODO if mempools has already closed this will FAIL!! */
- if (defined())
- memFreeString(size_, buf_);
-
- len_ = 0;
-
- size_ = 0;
-
- buf_ = NULL;
+ buf_.clear();
+ defined_=false;
+
PROF_stop(StringClean);
}
-String::~String()
-{
- clean();
-#if DEBUGSTRINGS
-
- StringRegistry::Instance().remove(this);
-#endif
-}
-
void
String::reset(char const *str)
{
PROF_start(StringReset);
clean(); // TODO: optimize to avoid cleaning the buffer if we can reuse it
- if (str)
- allocAndFill(str, strlen(str));
+ if (str) {
+ append(str);
+ // XXX: empty string "" sent to append means no change,
+ // but on reset / assignment it means set a defined empty string.
+ defined_=true;
+ }
PROF_stop(StringReset);
}
@@ -192,26 +134,8 @@
assert(str && len >= 0);
PROF_start(StringAppend);
- if (len_ + len < size_) {
- strncat(buf_, str, len);
- len_ += len;
- } else {
- // Create a temporary string and absorb it later.
- String snew;
- assert(len_ + len < 65536); // otherwise snew.len_ overflows below
- snew.len_ = len_ + len;
- snew.allocBuffer(snew.len_ + 1);
-
- if (len_)
- memcpy(snew.buf_, rawBuf(), len_);
-
- if (len)
- memcpy(snew.buf_ + len_, str, len);
-
- snew.buf_[snew.len_] = '\0';
-
- absorb(snew);
- }
+ buf_.append(str, len);
+ defined_ = true;
PROF_stop(StringAppend);
}
@@ -234,31 +158,24 @@
void
String::append(String const &old)
{
- append(old.rawBuf(), old.len_);
+ append(old.rawBuf(), old.size());
}
void
String::absorb(String &old)
{
- clean();
- setBuffer(old.buf_, old.size_);
- len_ = old.len_;
- old.size_ = 0;
- old.buf_ = NULL;
- old.len_ = 0;
+ operator = (old);
+ old.clean();
}
String
String::substr(String::size_type from, String::size_type to) const
{
-// Must(from >= 0 && from < size());
Must(from < size());
Must(to > 0 && to <= size());
Must(to > from);
- String rv;
- rv.limitInit(rawBuf()+from,to-from);
- return rv;
+ return String(buf_.substr(from, to-from));
}
#if DEBUGSTRINGS
@@ -442,7 +359,7 @@
const char *
String::pos(char const *aString) const
{
- if (undefined())
+ if (!defined())
return NULL;
return strstr(termedBuf(), aString);
}
@@ -450,7 +367,7 @@
const char *
String::pos(char const ch) const
{
- if (undefined())
+ if (!defined())
return NULL;
return strchr(termedBuf(), ch);
}
@@ -458,7 +375,7 @@
const char *
String::rpos(char const ch) const
{
- if (undefined())
+ if (!defined())
return NULL;
return strrchr(termedBuf(), (ch));
}
=== modified file 'src/String.cci'
--- src/String.cci 2013-11-23 00:58:42 +0000
+++ src/String.cci 2013-12-11 05:56:46 +0000
@@ -40,35 +40,36 @@
#endif /* INT_MAX */
#endif /* HAVE_STDINT_H */
-String::String() : size_(0), len_(0), buf_(NULL)
+String::String() : buf_(), defined_(false)
{
-#if DEBUGSTRINGS
- StringRegistry::Instance().add(this);
-#endif
}
String::size_type
String::size() const
{
- return len_;
+ return buf_.length();
}
char const *
String::rawBuf() const
{
- return buf_;
+ return termedBuf();
}
char const *
String::termedBuf() const
{
- return buf_;
+ // XXX: callers will probably try and write to the buffer,
+ // but SquidString never offered any more guarantee than SBuf
+ // does now about buffers existence.
+ // (this used to return NULL result on undefined)
+ return buf_.c_str();
}
char
String::operator [](unsigned int aPos) const
{
- assert(aPos < size_);
+ assert(aPos < size());
return buf_[aPos];
}
@@ -94,11 +95,7 @@
int
String::cmp(char const *aString) const
{
- int result = 0;
- if (nilCmp(!size(), (!aString || !*aString), result))
- return result;
-
- return strcmp(termedBuf(), aString);
+ return buf_.cmp(SBuf(aString));
}
int
@@ -114,21 +111,13 @@
int
String::cmp(String const &aString) const
{
- int result = 0;
- if (nilCmp(!size(), !aString.size(), result))
- return result;
-
- return strcmp(termedBuf(), aString.termedBuf());
+ return buf_.cmp(aString.buf_);
}
int
String::caseCmp(char const *aString) const
{
- int result = 0;
- if (nilCmp(!size(), (!aString || !*aString), result))
- return result;
-
- return strcasecmp(termedBuf(), aString);
+ return buf_.caseCmp(SBuf(aString));
}
int
@@ -144,44 +133,27 @@
int
String::caseCmp(const String &str) const
{
- return caseCmp(str.rawBuf(),str.size());
-}
-
-void
-String::set(char const *loc, char const ch)
-{
- if (loc < buf_ || loc > (buf_ + size_) ) return;
-
- buf_[loc-buf_] = ch;
+ return buf_.caseCmp(str.buf_);
}
void
String::cut(String::size_type newLength)
{
// size_type is size_t, unsigned. No need to check for newLength <0
- if (newLength > len_) return;
+ if (newLength > buf_.length()) return;
- len_ = newLength;
+ buf_ = buf_.substr(0, newLength);
// buf_ may be NULL on zero-length strings.
- if (len_ == 0 && buf_ == NULL) return;
-
- buf_[newLength] = '\0';
-}
-
-void
-String::cutPointer(char const *loc)
-{
- if (loc < buf_ || loc > (buf_ + size_) ) return;
-
- len_ = loc-buf_;
- buf_[len_] = '\0';
+ if (buf_.length() == 0 && !defined()) return;
+
+ buf_.setAt(newLength, '\0'); // terminate String.
}
std::ostream &
operator<<(std::ostream& os, String const &aString)
{
- os.write(aString.rawBuf(),aString.size());
+ os << aString.toSBuf();
return os;
}
=== modified file 'src/acl/HttpHeaderData.cc'
--- src/acl/HttpHeaderData.cc 2013-11-21 13:05:47 +0000
+++ src/acl/HttpHeaderData.cc 2013-12-19 08:07:16 +0000
@@ -76,8 +76,7 @@
return false;
}
- SBuf cvalue(value);
- return regex_rule->match(cvalue.c_str());
+ return regex_rule->match(value.termedBuf());
}
wordlist *
=== modified file 'src/acl/ReplyHeaderStrategy.h'
--- src/acl/ReplyHeaderStrategy.h 2013-10-25 00:13:46 +0000
+++ src/acl/ReplyHeaderStrategy.h 2013-12-11 05:29:21 +0000
@@ -67,7 +67,7 @@
{
char const *theHeader = checklist->reply->header.getStr(header);
- if (NULL == theHeader)
+ if (!theHeader || !*theHeader)
return 0;
return data->match(theHeader);
=== modified file 'src/acl/RequestHeaderStrategy.h'
--- src/acl/RequestHeaderStrategy.h 2013-10-25 00:13:46 +0000
+++ src/acl/RequestHeaderStrategy.h 2013-12-11 05:29:38 +0000
@@ -67,7 +67,7 @@
{
char const *theHeader = checklist->request->header.getStr(header);
- if (NULL == theHeader)
+ if (!theHeader || !*theHeader)
return 0;
return data->match(theHeader);
=== modified file 'src/adaptation/Iterator.cc'
--- src/adaptation/Iterator.cc 2013-12-05 11:04:45 +0000
+++ src/adaptation/Iterator.cc 2013-12-19 07:56:42 +0000
@@ -56,7 +56,7 @@
request = theCause;
Must(request);
Adaptation::History::Pointer ah = request->adaptHistory(true);
- SBuf gid(theGroup->id);
+ SBuf gid(theGroup->id.toSBuf());
ah->recordAdaptationService(gid);
}
@@ -96,7 +96,7 @@
if (Adaptation::Config::needHistory) {
Adaptation::History::Pointer ah = request->adaptHistory(true);
- SBuf uid(thePlan.current()->cfg().key);
+ SBuf uid(thePlan.current()->cfg().key.toSBuf());
ah->recordAdaptationService(uid);
}
=== modified file 'src/adaptation/ecap/ServiceRep.cc'
--- src/adaptation/ecap/ServiceRep.cc 2013-12-05 11:04:45 +0000
+++ src/adaptation/ecap/ServiceRep.cc 2013-12-19 07:56:42 +0000
@@ -235,7 +235,7 @@
bool Adaptation::Ecap::ServiceRep::wantsUrl(const String &urlPath) const
{
Must(up());
- return theService->wantsUrl(urlPath.termedBuf());
+ return theService->wantsUrl(urlPath);
}
Adaptation::Initiate *
=== modified file 'src/auth/UserRequest.cc'
--- src/auth/UserRequest.cc 2013-12-06 14:59:47 +0000
+++ src/auth/UserRequest.cc 2013-12-19 07:56:42 +0000
@@ -302,6 +302,10 @@
proxy_auth = request->header.getStr(headertype);
+ // handle empty string results
+ if (!*proxy_auth)
+ proxy_auth = NULL;
+
/*
* a note on proxy_auth logix here:
* proxy_auth==NULL -> unauthenticated request || already
=== modified file 'src/auth/digest/UserRequest.cc'
--- src/auth/digest/UserRequest.cc 2013-12-06 14:59:47 +0000
+++ src/auth/digest/UserRequest.cc 2013-12-19 07:56:42 +0000
@@ -146,7 +146,7 @@
if (last_broken_addr != request->client_addr) {
debugs(29, DBG_IMPORTANT, "Digest POST bug detected from "
<<
request->client_addr << " using '" <<
- (useragent ? useragent : "-") <<
+ (useragent && *useragent ? useragent : "-") <<
"'. Please upgrade browser. See Bug #630 for
details.");
last_broken_addr = request->client_addr;
=== modified file 'src/cache_manager.cc'
--- src/cache_manager.cc 2013-10-25 00:13:46 +0000
+++ src/cache_manager.cc 2013-12-11 03:56:15 +0000
@@ -148,7 +148,7 @@
Mgr::Action::Pointer
CacheManager::createNamedAction(const char *actionName)
{
- Must(actionName);
+ Must(actionName && *actionName);
Mgr::Command::Pointer cmd = new Mgr::Command;
cmd->profile = findAction(actionName);
=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc 2013-12-06 14:59:47 +0000
+++ src/client_side_request.cc 2013-12-19 07:56:42 +0000
@@ -604,7 +604,7 @@
// Require a Host: header.
const char *host = http->request->header.getStr(HDR_HOST);
- if (!host) {
+ if (!host || !*host) {
// TODO: dump out the HTTP/1.1 error about missing host header.
// otherwise this is fine, can't forge a header value when its not
even set.
debugs(85, 3, HERE << "validate skipped with no Host: header
present.");
@@ -1064,7 +1064,6 @@
HttpRequest *request = http->request;
HttpHeader *req_hdr = &request->header;
bool no_cache = false;
- const char *str;
request->imslen = -1;
request->ims = req_hdr->getTime(HDR_IF_MODIFIED_SINCE);
@@ -1090,7 +1089,8 @@
*/
if (Config.onoff.ie_refresh) {
if (http->flags.accel && request->flags.ims) {
- if ((str = req_hdr->getStr(HDR_USER_AGENT))) {
+ const char *str = req_hdr->getStr(HDR_USER_AGENT);
+ if (str && *str) {
if (strstr(str, "MSIE 5.01") != NULL)
no_cache=true;
else if (strstr(str, "MSIE 5.0") != NULL)
=== modified file 'src/esi/VarState.cc'
--- src/esi/VarState.cc 2012-11-19 11:29:31 +0000
+++ src/esi/VarState.cc 2013-12-11 05:42:34 +0000
@@ -409,7 +409,7 @@
ESIVariableUserAgent::esiUserOs_t
ESIVariableUserAgent::identifyOs(char const *s) const
{
- if (!s)
+ if (!s || !*s)
return ESI_OS_OTHER;
if (strstr (s, "Windows"))
=== modified file 'src/http.cc'
--- src/http.cc 2013-11-23 05:21:34 +0000
+++ src/http.cc 2013-12-19 07:56:42 +0000
@@ -451,7 +451,7 @@
* continuous push replies. These are generally dynamic and
* probably should not be cachable
*/
- if ((v = hdr->getStr(HDR_CONTENT_TYPE)))
+ if ((v = hdr->getStr(HDR_CONTENT_TYPE)) && *v)
if (!strncasecmp(v, "multipart/x-mixed-replace", 25)) {
debugs(22, 3, HERE << "NO because
Content-Type:multipart/x-mixed-replace");
return 0;
@@ -591,7 +591,6 @@
String vary, hdr;
const char *pos = NULL;
const char *item;
- const char *value;
int ilen;
static String vstr;
@@ -613,10 +612,9 @@
strListAdd(&vstr, name, ',');
hdr = request->header.getByName(name);
safe_free(name);
- value = hdr.termedBuf();
- if (value) {
- value = rfc1738_escape_part(value);
+ if (hdr.size()) {
+ const char *value = rfc1738_escape_part(hdr.termedBuf());
vstr.append("=\"", 2);
vstr.append(value);
vstr.append("\"", 1);
@@ -638,10 +636,9 @@
strListAdd(&vstr, name, ',');
hdr = request->header.getByName(name);
safe_free(name);
- value = hdr.termedBuf();
- if (value) {
- value = rfc1738_escape_part(value);
+ if (hdr.size()) {
+ const char *value = rfc1738_escape_part(hdr.termedBuf());
vstr.append("=\"", 2);
vstr.append(value);
vstr.append("\"", 1);
@@ -654,7 +651,7 @@
#endif
debugs(11, 3, "httpMakeVaryMark: " << vstr);
- return vstr.termedBuf();
+ return vstr.size() ? vstr.termedBuf() : NULL;
}
void
=== modified file 'src/redirect.cc'
--- src/redirect.cc 2013-11-23 00:58:42 +0000
+++ src/redirect.cc 2013-12-10 12:46:54 +0000
@@ -260,7 +260,7 @@
#endif
if (!r->client_ident && http->request->extacl_user.size() > 0) {
- r->client_ident = http->request->extacl_user.termedBuf();
+ r->client_ident = (http->request->extacl_user.size() ?
http->request->extacl_user.termedBuf() : NULL);
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-23 00:58:42 +0000
+++ src/ssl/ErrorDetail.cc 2013-12-02 09:59:30 +0000
@@ -473,7 +473,7 @@
static char tmpBuffer[64];
// We can use the GetErrorName but using the detailEntry is faster,
// so try it first.
- const char *err = detailEntry.name.termedBuf();
+ const char *err = (detailEntry.name.size() ? detailEntry.name.termedBuf()
: NULL);
// error details not loaded yet or not defined in error_details.txt,
// try the GetErrorName...
@@ -494,8 +494,8 @@
{
if (error_no == SSL_ERROR_NONE)
return "[No Error]";
- if (const char *err = detailEntry.descr.termedBuf())
- return err;
+ if (detailEntry.descr.size() > 0)
+ return detailEntry.descr.termedBuf();
return "[Not available]";
}
@@ -554,7 +554,7 @@
int code_len = 0;
if (ErrorDetailsManager::GetInstance().getErrorDetail(error_no, request,
detailEntry))
- s = detailEntry.detail.termedBuf();
+ s = (detailEntry.detail.size() ? detailEntry.detail.termedBuf() :
NULL);
if (!s)
s = SslErrorDetailDefaultStr;
=== modified file 'src/ssl/ErrorDetailManager.cc'
--- src/ssl/ErrorDetailManager.cc 2013-11-23 05:21:34 +0000
+++ src/ssl/ErrorDetailManager.cc 2013-12-19 07:56:42 +0000
@@ -133,7 +133,7 @@
errDetails = new ErrorDetailsList();
ErrorDetailFile detailTmpl(errDetails);
if (detailTmpl.loadFor(request.getRaw())) {
- if (detailTmpl.language()) {
+ if (detailTmpl.language() && *detailTmpl.language() != '\0') {
debugs(83, 8, HERE << "Found details on disk for language
" << detailTmpl.language());
errDetails->errLanguage = detailTmpl.language();
cacheDetails(errDetails);
=== modified file 'src/tests/testSBuf.cc'
--- src/tests/testSBuf.cc 2013-12-18 17:53:43 +0000
+++ src/tests/testSBuf.cc 2013-12-19 07:56:42 +0000
@@ -106,13 +106,6 @@
CPPUNIT_ASSERT_EQUAL(s4,s3);
}
- // TEST: go via SquidString adapters.
- {
- String str(fox);
- SBuf s1(str);
- CPPUNIT_ASSERT_EQUAL(literal,s1);
- }
-
// TEST: go via std::string adapter.
{
std::string str(fox);
=== modified file 'src/tests/testString.cc'
--- src/tests/testString.cc 2013-10-25 00:13:46 +0000
+++ src/tests/testString.cc 2013-11-20 22:04:34 +0000
@@ -37,13 +37,13 @@
String left("");
String right;
/* an empty string ("") is equal to a default string */
- CPPUNIT_ASSERT(!left.cmp(right));
- CPPUNIT_ASSERT(!left.cmp(NULL));
- CPPUNIT_ASSERT(!left.cmp(NULL, 1));
+ CPPUNIT_ASSERT_EQUAL(0,left.cmp(right));
+ CPPUNIT_ASSERT_EQUAL(0,left.cmp(NULL));
+ CPPUNIT_ASSERT_EQUAL(0,left.cmp(NULL, 1));
/* reverse the order to catch corners */
- CPPUNIT_ASSERT(!right.cmp(left));
- CPPUNIT_ASSERT(!right.cmp(""));
- CPPUNIT_ASSERT(!right.cmp("", 1));
+ CPPUNIT_ASSERT_EQUAL(0,right.cmp(left));
+ CPPUNIT_ASSERT_EQUAL(0,right.cmp(""));
+ CPPUNIT_ASSERT_EQUAL(0,right.cmp("", 1));
}
void
