Merge pull request #312 from TsudaKageyu/string-format

Unified the string formatting functions

taglib/mp4/mp4item.cpp

@@ -25,45 +25,16 @@
 
 #include "config.h"
 
-#include <cstdio>
-#include <cstdarg>
-
 #include "taglib.h"
 #include "tdebug.h"
 #include "tsmartptr.h"
 #include "mp4item.h"
+#include "tutils.h"
 
 using namespace TagLib;
 
 namespace
 {
-  String format(const char *fmt, ...)
-  {
-    va_list args;
-    va_start(args, fmt);
-
-    char buf[256];
-
-#if defined(HAVE_SNPRINTF)
-
-    vsnprintf(buf, sizeof(buf), fmt, args);
-
-#elif defined(HAVE_SPRINTF_S)
-
-    vsprintf_s(buf, fmt, args);
-
-#else
-
-    // Be careful. May cause a buffer overflow.  
-    vsprintf(buf, fmt, args);
-
-#endif
-
-    va_end(args);
-
-    return String(buf);
-  }
-
   struct ItemData
   {
     bool valid;
@@ -263,26 +234,26 @@ MP4::Item::toString() const
     case TypeBool:
       return d->data->m_bool ? "true" : "false";
     case TypeInt:
-      return format("%d", d->data->m_int);
+      return Utils::formatString("%d", d->data->m_int);
     case TypeIntPair:
-      return format("%d/%d", d->data->m_intPair.first, d->data->m_intPair.second);
+      return Utils::formatString("%d/%d", d->data->m_intPair.first, d->data->m_intPair.second);
     case TypeByte:
-      return format("%d", d->data->m_byte);
+      return Utils::formatString("%d", d->data->m_byte);
     case TypeUInt:
-      return format("%u", d->data->m_uint);
+      return Utils::formatString("%u", d->data->m_uint);
     case TypeLongLong:
-      return format("%lld", d->data->m_longlong);
+      return Utils::formatString("%lld", d->data->m_longlong);
     case TypeStringList:
       return d->data->m_stringList.toString(" / ");
     case TypeByteVectorList:
       for(TagLib::uint i = 0; i < d->data->m_byteVectorList.size(); i++) {
-        desc.append(format(
+        desc.append(Utils::formatString(
           "[%d bytes of data]", static_cast<int>(d->data->m_byteVectorList[i].size())));
       }
       return desc.toString(", ");
     case TypeCoverArtList:
       for(TagLib::uint i = 0; i < d->data->m_coverArtList.size(); i++) {
-        desc.append(format(
+        desc.append(Utils::formatString(
           "[%d bytes of data]", static_cast<int>(d->data->m_coverArtList[i].data().size())));
       }
       return desc.toString(", ");

taglib/toolkit/tdebug.cpp

@@ -29,44 +29,13 @@
 
 #include "tdebug.h"
 #include "tstring.h"
+#include "tutils.h"
 #include "tdebuglistener.h"
 
 #include <bitset>
-#include <cstdio>
-#include <cstdarg>
 
 using namespace TagLib;
 
-namespace
-{
-  String format(const char *fmt, ...)
-  {
-    va_list args;
-    va_start(args, fmt);
-
-    char buf[256];
-
-#if defined(HAVE_SNPRINTF)
-
-    vsnprintf(buf, sizeof(buf), fmt, args);
-
-#elif defined(HAVE_SPRINTF_S)
-
-    vsprintf_s(buf, fmt, args);
-
-#else
-
-    // Be careful. May cause a buffer overflow.  
-    vsprintf(buf, fmt, args);
-
-#endif
-
-    va_end(args);
-
-    return String(buf);
-  }
-}
-
 namespace TagLib
 {
   // The instance is defined in tdebuglistener.cpp.
@@ -88,7 +57,8 @@ namespace TagLib
     for(size_t i = 0; i < v.size(); ++i) 
     {
       std::string bits = std::bitset<8>(v[i]).to_string();
-      String msg = format("*** [%d] - char '%c' - int %d, 0x%02x, 0b%s\n", 
+      String msg = Utils::formatString(
+        "*** [%d] - char '%c' - int %d, 0x%02x, 0b%s\n", 
         i, v[i], v[i], v[i], bits.c_str());
 
       debugListener->printMessage(msg);

taglib/toolkit/tstring.cpp

@@ -36,7 +36,6 @@
 #include "tutils.h"
 
 #include <iostream>
-#include <cstdio>
 #include <cstring>
 
 #ifdef HAVE_STD_CODECVT
@@ -559,30 +558,7 @@ bool String::isAscii() const
 
 String String::number(int n) // static
 {
-  static const size_t BufferSize = 11; // Sufficient to store "-214748364".
-  static const char *Format = "%d";
-
-  char buffer[BufferSize];
-  int length;
-
-#if defined(HAVE_SNPRINTF)
-
-  length = snprintf(buffer, BufferSize, Format, n);
-
-#elif defined(HAVE_SPRINTF_S)
-
-  length = sprintf_s(buffer, Format, n);
-
-#else
-
-  length = sprintf(buffer, Format, n);
-
-#endif
-
-  if(length > 0)
-    return String(buffer);
-  else
-    return String::null;
+  return Utils::formatString("%d", n);
 }
 
 TagLib::wchar &String::operator[](size_t i)

taglib/toolkit/tutils.h

@@ -31,7 +31,7 @@
 #ifndef DO_NOT_DOCUMENT  // tell Doxygen not to document this header
 
 #ifdef HAVE_CONFIG_H
-#include <config.h>
+# include <config.h>
 #endif
 
 #if defined(HAVE_MSC_BYTESWAP)
@@ -44,6 +44,10 @@
 # include <sys/endian.h>
 #endif
 
+#include "tstring.h"
+#include <cstdio>
+#include <cstdarg>
+
 namespace TagLib
 {
   namespace Utils
@@ -144,6 +148,47 @@ namespace TagLib
 
 #endif
     }
+    
+    inline String formatString(const char *format, ...)
+    {
+      // Sufficient buffer size for the current internal uses.
+      // Consider changing this value when you use this function.
+
+      static const size_t BufferSize = 128; 
+
+      va_list args;
+      va_start(args, format);
+
+      char buf[BufferSize];
+      int length;
+
+#if defined(HAVE_SNPRINTF)
+
+      length = vsnprintf(buf, BufferSize, format, args);
+
+#elif defined(HAVE_SPRINTF_S)
+
+      length = vsprintf_s(buf, format, args);
+
+#else
+
+      // The last resort. May cause a buffer overflow.
+
+      length = vsprintf(buf, format, args);
+      if(length >= BufferSize) {
+        debug("Utils::formatString() - Buffer overflow! Returning an empty string.");
+        length = -1;
+      }
+
+#endif
+
+      va_end(args);
+
+      if(length != -1)
+        return String(buf);
+      else
+        return String::null;
+    }
 
 #ifdef SYSTEM_BYTEORDER
 

tests/test_string.cpp

@@ -41,6 +41,7 @@ class TestString : public CppUnit::TestFixture
   CPPUNIT_TEST(testAppendCharDetach);
   CPPUNIT_TEST(testAppendStringDetach);
   CPPUNIT_TEST(testToInt);
+  CPPUNIT_TEST(testFromInt);
   CPPUNIT_TEST(testSubstr);
   CPPUNIT_TEST(testNewline);
   CPPUNIT_TEST(testUpper);
@@ -216,6 +217,12 @@ public:
     CPPUNIT_ASSERT_EQUAL(String("-123aa").toInt(), -123);
   }
 
+  void testFromInt()
+  {
+    CPPUNIT_ASSERT_EQUAL(String("123"), String::number(123));
+    CPPUNIT_ASSERT_EQUAL(String("-123"), String::number(-123));
+  }
+
   void testSubstr()
   {
     CPPUNIT_ASSERT_EQUAL(String("01"), String("0123456").substr(0, 2));