[binaryToString] Reimplement using Slices

- reimplement binaryToString:
  - it now accepts a Slice and nothing else
  - it does not return a std::string but instead a proxy object that implements
    operator<< (this should be more efficient, as we do not need to touch the
    heap in most cases)
- addapt unit tests
- replace all occurences with the new API

src/bigtiffimage.cpp

@@ -334,7 +334,7 @@ namespace Exiv2
                                     }
                                 }
                                 else if ( isStringType(type) )
-                                    out << sp << Internal::binaryToString(buf, (size_t) kount);
+                                    out << sp << Internal::binaryToString(makeSlice(buf, 0, static_cast<size_t>(kount)));
 
                                 sp = kount == count ? "" : " ...";
                                 out << sp << std::endl;

src/image.cpp

@@ -440,7 +440,7 @@ namespace Exiv2 {
                             sp = " ";
                         }
                     } else if ( isStringType(type) ) {
-                        out << sp << Internal::binaryToString(buf, kount);
+                        out << sp << Internal::binaryToString(makeSlice(buf, 0, kount));
                     }
 
                     sp = kount == count ? "" : " ...";

src/image_int.cpp

@@ -61,31 +61,6 @@ namespace Exiv2
             return result;
         }
 
-        std::string binaryToString(const byte* buff, size_t size, size_t start /*=0*/)
-        {
-            std::string result = "";
-            size += start;
-
-            while (start < size) {
-                int c = (int)buff[start++];
-                bool bTrailingNull = c == 0 && start == size;
-                if (!bTrailingNull) {
-                    if (c < ' ' || c >= 127)
-                        c = '.';
-                    result += (char)c;
-                }
-            }
-            return result;
-        }
-
-        std::string binaryToString(const DataBuf& buf, size_t size, size_t start /*=0*/)
-        {
-            if (size + start > static_cast<size_t>(buf.size_)) {
-                size = static_cast<size_t>(buf.size_) - start;
-            }
-            return binaryToString(buf.pData_, size, start);
-        }
-
         std::string binaryToHex(const byte* data, size_t size)
         {
             std::stringstream hexOutput;

src/image_int.hpp

@@ -54,35 +54,78 @@ namespace Exiv2 {
     std::string stringFormat(const char* format, ...) ATTRIBUTE_FORMAT_PRINTF;
 
     /*!
-     * @brief format data binary for display in @ref Image::printStructure()
+     * @brief Helper struct for binary data output via @ref binaryToString.
      *
-     * Overload for DataBuf.
+     * The only purpose of this struct is to provide a custom
+     * `operator<<(std::ostream&)` for the output of binary data, that is not
+     * used for all Slices by default.
      */
-    std::string binaryToString(const DataBuf& buf, size_t size, size_t start =0);
+    template <typename T>
+    struct binaryToStringHelper;
 
     /*!
-     * @brief format data binary for display in @ref Image::printStructure()
+     * @brief Actual implementation of the output algorithm described in @ref
+     * binaryToString
      *
-     * This function creates printable version of the binary data in `buff`
-     * according to the following rules:
+     * @throws nothing
+     */
+    template <typename T>
+    std::ostream& operator<<(std::ostream& stream, const binaryToStringHelper<T>& binToStr) throw()
+    {
+        for (size_t i = 0; i < binToStr.buf_.size(); ++i) {
+            int c = static_cast<int>(binToStr.buf_.at(i));
+            const bool bTrailingNull = c == 0 && i == binToStr.buf_.size() - 1;
+            if (!bTrailingNull) {
+                if (c < ' ' || c >= 127) {
+                    c = '.';
+                }
+                stream.put(static_cast<char>(c));
+            }
+        }
+        return stream;
+    }
+
+    template <typename T>
+    struct binaryToStringHelper
+    {
+        explicit binaryToStringHelper(const Slice<T> buf) throw() : buf_(buf)
+        {
+        }
+
+        friend std::ostream& operator<<<T>(std::ostream& stream, const binaryToStringHelper<T>& binToStr) throw();
+
+        // the Slice is stored by value to avoid dangling references, in case we
+        // invoke:
+        // binaryToString(makeSlice(buf, 0, n));
+        // <- buf_ would be now dangling, were it a reference
+        const Slice<T> buf_;
+    };
+
+    /*!
+     * @brief format binary data for display in @ref Image::printStructure()
+     *
+     * This function creates a new helper class that can be passed to a
+     * `std::ostream` for output. It creates a printable version of the binary
+     * data in the slice sl according to the following rules:
      * - characters with numeric values larger than 0x20 (= space) and smaller
      *   or equal to 0x7F (Delete) are printed as ordinary characters
      * - characters outside of that range are printed as '.'
-     * - if the last element of the buffer is 0, then it is omitted
+     * - if the last element of the slice is 0, then it is omitted
      *
-     * @param[in] buff  Binary data buffer that should be printed. Must have
-     * length `size + start`.
-     * @param[in] size  Number of bytes from buffer that will be converted to
-     * the printable version. This is **not** the length of the buffer!
-     * @param[in] start  Begin of the region of buff that will be printed. The
-     * region ends at `start + size`.
+     * @param[in] sl  Slice containing binary data buffer that should be
+     *     printed.
      *
-     * **CAUTION** In contrast to the expected behavior, the second parameter is
-     * **not** the length of `buff` but the length of the printed region.
+     * @return Helper object, that can be passed into a std::ostream and
+     *     produces an output according to the aforementioned rules.
      *
-     * @return Appropriately formatted string
+     * @throw This function does not throw. The output of the helper object to
+     *     the stream throws neither.
      */
-    std::string binaryToString(const byte* buff, size_t size, size_t start /*=0*/);
+    template <typename T>
+    inline binaryToStringHelper<T> binaryToString(const Slice<T> sl) throw()
+    {
+        return binaryToStringHelper<T>(sl);
+    }
 
     /*!
       @brief format binary for display of raw data .

src/iptc.cpp

@@ -365,7 +365,8 @@ namespace Exiv2 {
             sprintf(buff, "  %6d | %7d | %-24s | %6d | ", record, dataset,
                     Exiv2::IptcDataSets::dataSetName(dataset, record).c_str(), len);
 
-            out << buff << Internal::binaryToString(bytes, (len > 40 ? 40 : len), i + 5) << (len > 40 ? "..." : "")
+            out << buff << Internal::binaryToString(makeSlice(bytes, i + 5, i + 5 + (len > 40 ? 40 : len)))
+                << (len > 40 ? "..." : "")
                 << std::endl;
             i += 5 + len;
         }

src/jp2image.cpp

@@ -511,7 +511,7 @@ namespace Exiv2
                             io_->read(data.pData_,data.size_);
                             if ( bPrint ) {
                                 out << Internal::stringFormat("%8ld | %8ld |  sub:",address,subBox.length) << toAscii(subBox.type)
-                                <<" | " << Internal::binaryToString(data,30,0);
+                                    <<" | " << Internal::binaryToString(makeSlice(data, 0, 30));
                                 bLF = true;
                             }
 
@@ -559,7 +559,7 @@ namespace Exiv2
                             if (bufRead != rawData.size_) throw Error(kerInputDataReadFailed);
 
                             if ( bPrint ){
-                                out << Internal::binaryToString(rawData,40,0);
+                                out << Internal::binaryToString(makeSlice(rawData,0,40));
                                 out.flush();
                             }
                             lf(out,bLF);

src/jpgimage.cpp

@@ -709,7 +709,9 @@ namespace Exiv2 {
                             iptcDataSegs.push_back(size);
                         }
                     } else if (bPrint) {
-                        out << "| " << Internal::binaryToString(buf, size > 32 ? 32 : size, size > 0 ? 2 : 0);
+                        const size_t start = size > 0 ? 2 : 0;
+                        const size_t end = start + (size > 32 ? 32 : size);
+                        out << "| " << Internal::binaryToString(makeSlice(buf, start, end));
                         if (signature.compare(iccId_) == 0) {
                             // extract the chunk information from the buffer
                             //
@@ -796,8 +798,12 @@ namespace Exiv2 {
 
                 // print COM marker
                 if (bPrint && marker == com_) {
-                    int n = (size - 2) > 32 ? 32 : size - 2;             // size includes 2 for the two bytes for size!
-                    out << "| " << Internal::binaryToString(buf, n, 2);  // start after the two bytes
+                    // size includes 2 for the two bytes for size!
+                    const int n = (size - 2) > 32 ? 32 : size - 2;
+                    // start after the two bytes
+                    out << "| "
+                        << Internal::binaryToString(
+                               makeSlice(buf, 2, n + 2 /* cannot overflow as n is at most size - 2 */));
                 }
 
                 // Skip the segment if the size is known

src/pngimage.cpp

@@ -274,8 +274,14 @@ namespace Exiv2 {
 
                 // format output
                 const int    iMax = 30 ;
-                uint32_t     blen = dataOffset > iMax ? iMax : dataOffset ;
-                std::string  dataString = Internal::binaryToString(buff, blen);
+                const uint32_t blen = dataOffset > iMax ? iMax : dataOffset ;
+                std::string dataString = "";
+                // if blen == 0 => slice construction fails
+                if (blen > 0) {
+                    std::stringstream ss;
+                    ss << Internal::binaryToString(makeSlice(buff, 0, blen));
+                    dataString = ss.str();
+                }
                 while (      dataString.size() < iMax ) dataString += ' ';
                 dataString = dataString.substr(0,iMax);
 

src/rafimage.cpp

@@ -248,7 +248,7 @@ namespace Exiv2 {
                 out << Internal::indent(depth)
                     << Internal::stringFormat("  %8u | %8u | ", jpg_img_len, jpg_img_off)
                     << "jpg image / exif : "
-                    << Internal::binaryToString(payload, payload.size_)
+                    << Internal::binaryToString(makeSlice(payload, 0, payload.size_))
                     << std::endl;
             }
 
@@ -258,7 +258,7 @@ namespace Exiv2 {
                 out << Internal::indent(depth)
                     << Internal::stringFormat("  %8u | %8u | ", cfa_hdr_len, cfa_hdr_off)
                     << "CFA Header: "
-                    << Internal::binaryToString(payload, payload.size_)
+                    << Internal::binaryToString(makeSlice(payload, 0, payload.size_))
                     << std::endl;
             }
 
@@ -268,7 +268,7 @@ namespace Exiv2 {
                 out << Internal::indent(depth)
                     << Internal::stringFormat("  %8u | %8u | ", cfa_len, cfa_off)
                     << "CFA : "
-                    << Internal::binaryToString(payload, payload.size_)
+                    << Internal::binaryToString(makeSlice(payload, 0, payload.size_))
                     << std::endl;
             }
         }

src/riffvideo.cpp

@@ -571,7 +571,7 @@ namespace Exiv2 {
                 if ( bPrint ) {
                     out << Internal::indent(depth)
                         << Internal::stringFormat("  %s | %12u | %12u | ", (const char*)chunkId.pData_,size,(uint32_t)offset)
-                        << Internal::binaryToString(payload,payload.size_>32?32:payload.size_)
+                        << Internal::binaryToString(makeSlice(payload, 0, payload.size_ > 32 ? 32 : payload.size_))
                         << std::endl;
                 }
 

src/webpimage.cpp

@@ -449,7 +449,7 @@ namespace Exiv2 {
                 if ( bPrint ) {
                     out << Internal::indent(depth)
                     << Internal::stringFormat("  %s | %8u | %8u | ", (const char*)chunkId.pData_,size,(uint32_t)offset)
-                    << Internal::binaryToString(payload,payload.size_>32?32:payload.size_)
+                    << Internal::binaryToString(makeSlice(payload, 0, payload.size_ > 32 ? 32 : payload.size_))
                     << std::endl;
                 }
 

unitTests/test_image_int.cpp

@@ -3,46 +3,46 @@
 #include <image_int.hpp>
 
 using namespace Exiv2::Internal;
+using Exiv2::makeSlice;
+using Exiv2::Slice;
 
 static const unsigned char buf[10] = {'a', 'b', 'c', 1, 4, 0, 'e', 136, 0, 'a'};
 
+template <typename T>
+void checkBinaryToString(const Exiv2::Slice<T> sl, const char* expectedOutput)
+{
+    // construct the helper manually so that we catch potential invalidation of
+    // temporaries
+    std::stringstream ss;
+    const binaryToStringHelper<T> helper = binaryToString(sl);
+    ss << helper;
+
+    ASSERT_STREQ(ss.str().c_str(), expectedOutput);
+}
+
 TEST(binaryToString, zeroStart)
 {
     // a, b, c are printable, 1 & 4 are not => '.', 0 at last position => skipped
-    ASSERT_STREQ(binaryToString(buf, 6, 0).c_str(), "abc..");
+    checkBinaryToString(makeSlice(buf, 0, 6), "abc..");
 
     // same as previous, but now last element is not ignored since it is not 0
-    ASSERT_STREQ(binaryToString(buf, 5, 0).c_str(), "abc..");
+    checkBinaryToString(makeSlice(buf, 0, 5), "abc..");
 
     // same as first, only now the 0 & 136 are converted to '.'
-    ASSERT_STREQ(binaryToString(buf, 8, 0).c_str(), "abc...e.");
+    checkBinaryToString(makeSlice(buf, 0, 8), "abc...e.");
 
     // should result in the same as previously, as trailing zero is ignored
-    ASSERT_STREQ(binaryToString(buf, 9, 0).c_str(), "abc...e.");
+    checkBinaryToString(makeSlice(buf, 0, 9), "abc...e.");
 
     // ensure that the function does not overread when last element != 0
-    ASSERT_STREQ(binaryToString(buf, sizeof(buf), 0).c_str(), "abc...e..a");
+    checkBinaryToString(makeSlice(buf, 0, sizeof(buf)), "abc...e..a");
 }
 
 TEST(binaryToString, nonZeroStart)
 {
     // start @ index 1, read 6 characters (until e)
-    ASSERT_STREQ(binaryToString(buf, 6, 1).c_str(), "bc...e");
+    checkBinaryToString(makeSlice(buf, 1, 7), "bc...e");
 
     // start @ index 3, read until end
-    ASSERT_STREQ(binaryToString(buf, sizeof(buf) - 3, 3).c_str(), "...e..a");
-}
-
-TEST(binaryToString, DataBuf)
-{
-    const Exiv2::DataBuf data_buf(buf, sizeof(buf));
-
-    // same as first case in zeroStart
-    ASSERT_EQ(binaryToString(data_buf, 6, 0), "abc..");
-
-    // try to pass a too large value for size
-    ASSERT_EQ(binaryToString(data_buf, 200, 0), "abc...e..a");
-
-    // make it blow up by setting start larger than zero...
-    ASSERT_EQ(binaryToString(data_buf, 200, 1), "bc...e..a");
+    checkBinaryToString(makeSlice(buf, 3, sizeof(buf)), "...e..a");
 }