Fixed documentation, tentative alternate dump implementation with Raw api

Improve SBuf::find readability
Improved readability and error reporting in SBuf
Fixed nonfatal bug in snprintf behavior detection

diff --git a/src/SBuf.cc b/src/SBuf.cc
index 48050976548152b22ac42ea1c78e36adc604f04b..a23b5a57ca9d8578544e9ceb9280f2ea04e8b98f 100644 (file)
--- a/src/SBuf.cc
+++ b/src/SBuf.cc
@@ -295,6 +295,9 @@ SBuf::vappendf(const char *fmt, va_list vargs)
         requiredSpaceEstimate = sz*2; // TODO: tune heuristics
         space = rawSpace(requiredSpaceEstimate);
         sz = vsnprintf(space, spaceSize(), fmt, vargs);
+        if (sz < 0) // output error in vsnprintf
+            throw TextException("output error in second-go vsnprintf",__FILE__,
+                __LINE__);
     }
 
     if (sz < 0) // output error in either vsnprintf
@@ -303,16 +306,16 @@ SBuf::vappendf(const char *fmt, va_list vargs)
     // data was appended, update internal state
     len_ += sz;
 
-
     /* C99 specifies that the final '\0' is not counted in vsnprintf's
      * return value. Older compilers/libraries might instead count it */
     /* check whether '\0' was appended and counted */
-    static bool snPrintfTerminatorChecked = false,
-                snPrintfTerminatorCounted = false;
+    static bool snPrintfTerminatorChecked = false;
+    static bool snPrintfTerminatorCounted = false;
     if (!snPrintfTerminatorChecked) {
         char testbuf[16];
         snPrintfTerminatorCounted = snprintf(testbuf, sizeof(testbuf),
             "%s", "1") == 2;
+        snPrintfTerminatorChecked = true;
     }
     if (snPrintfTerminatorCounted) {
         --sz;
@@ -339,12 +342,23 @@ SBuf::dump(std::ostream &os) const
     os << id
     << ": ";
     store_->dump(os);
-    os << ",offset:" << off_
-    << ",len:" << len_
+    os << ", offset:" << off_
+    << ", len:" << len_
     << ") : '";
     print(os);
     os << '\'' << std::endl;
     return os;
+# if 0
+    // alternate implementation, based on Raw() API.
+    os << Raw("SBuf", buf(), length()) <<
+    ". id: " << id <<
+    ", offset:" << off_ <<
+    ", len:" << len_ <<
+    ", store: ";
+    store_->dump(os);
+    os << std::endl;
+    return os;
+#endif
 }
 
 void
@@ -375,11 +389,9 @@ int
 SBuf::compare(const SBuf &S, SBufCaseSensitive isCaseSensitive, size_type n) const
 {
     Must(n == npos || n >= 0);
-    if (n != npos) {
-        if (n > length())
-            return compare(S.substr(0,n),isCaseSensitive);
+    if (n != npos)
         return substr(0,n).compare(S.substr(0,n),isCaseSensitive);
-    }
+
     size_type byteCompareLen = min(S.length(), length());
     ++stats.compareSlow;
     int rv = 0;
@@ -553,13 +565,11 @@ SBuf::find(char c, size_type startPos) const
 {
     ++stats.find;
 
-    // for npos with char sd::string returns npos
-    // this differs from how std::string handles 1-length string
-    if (startPos == npos)
+    if (startPos == npos) // can't find anything if we look past end of SBuf
         return npos;
 
     // std::string returns npos if needle is outside hay
-    if (startPos >= length())
+    if (startPos > length())
         return npos;
 
     // ignore invalid startPos
@@ -577,8 +587,16 @@ SBuf::find(char c, size_type startPos) const
 SBuf::size_type
 SBuf::find(const SBuf &needle, size_type startPos) const
 {
+    if (startPos == npos) { // can't find anything if we look past end of SBuf
+        ++stats.find;
+        return npos;
+    }
+
+    if (startPos < 0)
+        startPos = 0;
+
     // std::string allows needle to overhang hay but not start outside
-    if (startPos != npos && startPos > length()) {
+    if (startPos > length()) {
         ++stats.find;
         return npos;
     }
@@ -589,11 +607,6 @@ SBuf::find(const SBuf &needle, size_type startPos) const
         return startPos;
     }
 
-    // for npos with char* std::string scans entire hay
-    // this differs from how std::string handles single char from npos
-    if (startPos == npos)
-        return npos;
-
     // if needle length is 1 use the char search
     if (needle.length() == 1)
         return find(needle[0], startPos);
@@ -637,21 +650,16 @@ SBuf::rfind(const SBuf &needle, SBuf::size_type endPos) const
 
     // on npos input std::string scans from the end of hay
     if (endPos == npos || endPos > length())
-        endPos=length();
+        endPos = length();
 
     // on empty hay std::string returns npos
     if (length() < needle.length())
         return npos;
 
-    // on empty needle std::string returns the position the search starts
+    // consistent with std::string: on empty needle return min(endpos,length())
     if (needle.length() == 0)
         return endPos;
 
-/* std::string permits needle to overhang endPos
-    if (endPos <= needle.length())
-        return npos;
-*/
-
     char *bufBegin = buf();
     char *cur = bufBegin+endPos;
     char needleBegin = needle[0];
@@ -672,7 +680,7 @@ SBuf::rfind(char c, SBuf::size_type endPos) const
 {
     ++stats.find;
 
-    // on empty hay std::string returns size of hay
+    // shortcut: haystack is empty, can't find anything by definition
     if (length() == 0)
         return npos;
 
@@ -689,6 +697,9 @@ SBuf::rfind(char c, SBuf::size_type endPos) const
         ++endPos;
     }
 
+    if (length() == 0)
+        return endPos;
+
     const void *i = memrchr(buf(), (int)c, (size_type)endPos);
 
     if (i == NULL)
@@ -709,7 +720,7 @@ SBuf::find_first_of(const SBuf &set, size_type startPos) const
     if (startPos == npos)
         return npos;
 
-    if (startPos > length())
+    if (startPos >= length())
         return npos;
 
     if (startPos < 0)
@@ -789,7 +800,7 @@ SBuf::toLower() const
         if (isupper(c))
             rv.setAt(j, tolower(c)); //will cow() if needed
     }
-    debugs(24, 8, "result: \"" << *this << "\"");
+    debugs(24, 8, "result: \"" << rv << "\"");
     ++stats.caseChange;
     return rv;
 }
@@ -804,7 +815,7 @@ SBuf::toUpper() const
         if (islower(c))
             rv.setAt(j, toupper(c)); //will cow() if needed
     }
-    debugs(24, 8, "result: \"" << *this << "\"");
+    debugs(24, 8, "result: \"" << rv << "\"");
     ++stats.caseChange;
     return rv;
 }
@@ -842,7 +853,7 @@ SBuf::toString() const
 void
 SBuf::reAlloc(size_type newsize)
 {
-    debugs(24, DBG_DATA, "new size: " << newsize);
+    debugs(24, 8, "new size: " << newsize);
     if (newsize > maxSize)
         throw SBufTooBigException(__FILE__, __LINE__);
     MemBlob::Pointer newbuf = new MemBlob(newsize);
@@ -868,21 +879,18 @@ SBuf::lowAppend(const char * memArea, size_type areaSize)
  * copy-on-write: make sure that we are the only holder of the backing store.
  * If not, reallocate. If a new size is specified, and it is greater than the
  * current length, the backing store will be extended as needed
- * \retval false no grow was needed
- * \retval true had to copy
  */
-bool
+void
 SBuf::cow(SBuf::size_type newsize)
 {
-    debugs(24, DBG_DATA, "new size:" << newsize);
+    debugs(24, 8, "new size:" << newsize);
     if (newsize == npos || newsize < length())
         newsize = length();
 
     if (store_->LockCount() == 1 && newsize == length()) {
-        debugs(24, DBG_DATA, "no cow needed");
+        debugs(24, 8, "no cow needed");
         ++stats.cowFast;
-        return false;
+        return;
     }
     reAlloc(newsize);
-    return true;
 }

diff --git a/src/SBuf.h b/src/SBuf.h
index 0752223851ef8148fd5140b75d3fe2383d54fdb7..dc030b64cbf181af49106e7c2b340fc416bd1746 100644 (file)
--- a/src/SBuf.h
+++ b/src/SBuf.h
@@ -194,9 +194,9 @@ public:
      * as needed.
      *
      * \param S the c string to be copied. Can be NULL.
-     * \param pos how many bytes to skip at the beginning of the c-string
-     * \param n how many bytes to import into the SBuf. If it is npos
-     *              or unspecified, imports to end-of-cstring
+     * \param Ssize how many bytes to import into the SBuf. If it is npos
+     *              or unspecified, imports to end-of-cstring. If S is NULL,
+     *              Ssize is ignored.
      * \note to append a std::string use the pattern
      *     cstr_append(stdstr.data(), stdstd.length())
      */
@@ -591,7 +591,7 @@ private:
 
     void reAlloc(size_type newsize);
 
-    bool cow(size_type minsize = npos);
+    void cow(size_type minsize = npos);
 
     void checkAccessBounds(size_type pos) const;
 

diff --git a/src/SBufExceptions.cc b/src/SBufExceptions.cc
index f13e6438c7ebd59f93f5c4879586ce55af81e3fe..01106c4d01fb50633fa6f68c68afaa5376c54df0 100644 (file)
--- a/src/SBufExceptions.cc
+++ b/src/SBufExceptions.cc
@@ -55,10 +55,6 @@ OutOfBoundsException::OutOfBoundsException(const SBuf &throwingBuf,
 OutOfBoundsException::~OutOfBoundsException() throw()
 { }
 
-NullSBufException::NullSBufException(const char *aFilename, int aLineNo)
-        : TextException("Trying to access a null SBuf", aFilename, aLineNo)
-{ }
-
 InvalidParamException::InvalidParamException(const char *aFilename, int aLineNo)
         : TextException("Invalid parameter", aFilename, aLineNo)
 { }
@@ -66,5 +62,3 @@ InvalidParamException::InvalidParamException(const char *aFilename, int aLineNo)
 SBufTooBigException::SBufTooBigException(const char *aFilename, int aLineNo)
         : TextException("Trying to create an oversize SBuf", aFilename, aLineNo)
 { }
-
-/* */

diff --git a/src/SBufExceptions.h b/src/SBufExceptions.h
index e30761923a3a843d88f326505c055f48e67ba7b9..70ec2a76a658a395a0095e9cd45bb664fee43c8b 100644 (file)
--- a/src/SBufExceptions.h
+++ b/src/SBufExceptions.h
@@ -33,16 +33,6 @@
 
 #include "base/TextException.h"
 
-/**
- * Exception raised when the user is trying to operate on a Null SBuf
- * \todo move to an Exceptions.h?
- */
-class NullSBufException : public TextException
-{
-public:
-    explicit NullSBufException(const char *aFilename = 0, int aLineNo = -1);
-};
-
 /**
  * Exception raised when call parameters are not valid
  * \todo move to an Exceptions.h?

diff --git a/src/SBufStream.h b/src/SBufStream.h
index 63331f130e68fe846bcfa49f99d949a4a27cf1b6..d6574e9fbc9da5f4a1ef2b408bc0ead64e00ff23 100644 (file)
--- a/src/SBufStream.h
+++ b/src/SBufStream.h
@@ -126,11 +126,13 @@ public:
 
     /// Retrieve a copy of the current stream status
     SBuf buf() {
+        flush();
         return theBuffer.getBuf();
     }
 
     /// Clear the stream's backing store
     SBufStream& clearBuf() {
+        flush();
         theBuffer.clearBuf();
         return *this;
     }

diff --git a/src/tests/testSBuf.cc b/src/tests/testSBuf.cc
index 432093a6f12b382680bc32fbcc370c748aa13164..2a796c7891d5afde21905ef2a9c827e6af92bafb 100644 (file)
--- a/src/tests/testSBuf.cc
+++ b/src/tests/testSBuf.cc
@@ -397,7 +397,7 @@ testSBuf::testChomp()
     CPPUNIT_ASSERT_EQUAL(s1,s2);
 }
 
-// inspired to SBufFindTest; to be expanded.
+// inspired by SBufFindTest; to be expanded.
 class SBufSubstrAutoTest
 {
     SBuf fullString, sb;