From 37cf7314a03bd6ee05fbd0b89e16d563e8dc6769 Mon Sep 17 00:00:00 2001
From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Thu, 7 Aug 2014 11:13:26 -0600
Subject: [PATCH] Do not allow SBuf::toLower/toUpper() return a value.

Some callers think toLower() method changes the string and some think it
returns a changed value without changing the string. Without unportable hacks
like __attribute__((warn_unused_result)), some of the callers will guess wrong
and not know about it. It is easier/safer to change the object itself and
return nothing.

Provided ToUpper/ToLower() functions (not methods!) that preserve their
paramter and return a new object for callers that need the old functionality.
These functions are a good candidate for __attribute__ hacks.
---
 src/SBuf.cc            | 20 ++++++++------------
 src/SBuf.h             | 32 ++++++++++++++++++++------------
 src/tests/stub_SBuf.cc |  4 ++--
 src/tests/testSBuf.cc  |  4 ++--
 4 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/src/SBuf.cc b/src/SBuf.cc
index 3fd780b10c..47acbed3ef 100644
--- a/src/SBuf.cc
+++ b/src/SBuf.cc
@@ -840,34 +840,30 @@ SBufStats::dump(std::ostream& os) const
     return os;
 }
 
-SBuf
-SBuf::toLower() const
+void
+SBuf::toLower()
 {
     debugs(24, 8, "\"" << *this << "\"");
-    SBuf rv(*this);
     for (size_type j = 0; j < length(); ++j) {
         const int c = (*this)[j];
         if (isupper(c))
-            rv.setAt(j, tolower(c)); //will cow() if needed
+            setAt(j, tolower(c));
     }
-    debugs(24, 8, "result: \"" << rv << "\"");
+    debugs(24, 8, "result: \"" << *this << "\"");
     ++stats.caseChange;
-    return rv;
 }
 
-SBuf
-SBuf::toUpper() const
+void
+SBuf::toUpper()
 {
     debugs(24, 8, "\"" << *this << "\"");
-    SBuf rv(*this);
     for (size_type j = 0; j < length(); ++j) {
         const int c = (*this)[j];
         if (islower(c))
-            rv.setAt(j, toupper(c)); //will cow() if needed
+            setAt(j, toupper(c));
     }
-    debugs(24, 8, "result: \"" << rv << "\"");
+    debugs(24, 8, "result: \"" << *this << "\"");
     ++stats.caseChange;
-    return rv;
 }
 
 /**
diff --git a/src/SBuf.h b/src/SBuf.h
index dec61489be..2cde1c8825 100644
--- a/src/SBuf.h
+++ b/src/SBuf.h
@@ -546,19 +546,11 @@ public:
      */
     int scanf(const char *format, ...);
 
-    /** Lower-case SBuf
-     *
-     * Returns a lower-cased COPY of the SBuf
-     * \see man tolower(3)
-     */
-    SBuf toLower() const;
+    /// converts all characters to lower case; \see man tolower(3)
+    void toLower();
 
-    /** Upper-case SBuf
-     *
-     * Returns an upper-cased COPY of the SBuf
-     * \see man toupper(3)
-     */
-    SBuf toUpper() const;
+    /// converts all characters to upper case; \see man toupper(3)
+    void toUpper();
 
     /** String export function
      * converts the SBuf to a legacy String, by copy.
@@ -631,4 +623,20 @@ operator <<(std::ostream& os, const SBuf& S)
     return S.print(os);
 }
 
+/// Returns a lower-cased copy of its parameter.
+inline SBuf
+ToUpper(SBuf buf)
+{
+    buf.toUpper();
+    return buf;
+}
+
+/// Returns an upper-cased copy of its parameter.
+inline SBuf
+ToLower(SBuf buf)
+{
+    buf.toLower();
+    return buf;
+}
+
 #endif /* SQUID_SBUF_H */
diff --git a/src/tests/stub_SBuf.cc b/src/tests/stub_SBuf.cc
index 66e8713fd6..dc9ec145cc 100644
--- a/src/tests/stub_SBuf.cc
+++ b/src/tests/stub_SBuf.cc
@@ -55,6 +55,6 @@ SBuf::size_type SBuf::rfind(const SBuf &str, size_type endPos) const STUB_RETVAL
 SBuf::size_type SBuf::findFirstOf(const CharacterSet &set, size_type startPos) const STUB_RETVAL(SBuf::npos)
 SBuf::size_type SBuf::findFirstNotOf(const CharacterSet &set, size_type startPos) const STUB_RETVAL(SBuf::npos)
 int SBuf::scanf(const char *format, ...) STUB_RETVAL(-1)
-SBuf SBuf::toLower() const STUB_RETVAL(*this)
-SBuf SBuf::toUpper() const STUB_RETVAL(*this)
+void SBuf::toLower() STUB
+void SBuf::toUpper() STUB
 String SBuf::toString() const STUB_RETVAL(String(""))
diff --git a/src/tests/testSBuf.cc b/src/tests/testSBuf.cc
index 9b18a5d83e..19a0931470 100644
--- a/src/tests/testSBuf.cc
+++ b/src/tests/testSBuf.cc
@@ -784,7 +784,7 @@ testSBuf::testCopy()
 void
 testSBuf::testStringOps()
 {
-    SBuf sng(literal.toLower()),
+    SBuf sng(ToLower(literal)),
     ref("the quick brown fox jumped over the lazy dog");
     CPPUNIT_ASSERT_EQUAL(ref,sng);
     sng=literal;
@@ -820,7 +820,7 @@ testSBuf::testStartsWith()
 
     // case-insensitive checks
     CPPUNIT_ASSERT(literal.startsWith(casebuf,caseInsensitive));
-    casebuf=SBuf(fox1).toUpper();
+    casebuf=ToUpper(SBuf(fox1));
     CPPUNIT_ASSERT(literal.startsWith(casebuf,caseInsensitive));
     CPPUNIT_ASSERT(literal.startsWith(SBuf(fox1),caseInsensitive));
     casebuf = "tha quick";
-- 
2.47.3