Rewrite QUtil::int_to_string and QUtil::double_to_string

Make them safer by avoiding any internal limits and replacing sprintf
with std::ostringstream.

5 files changed, 41 insertions, 129 deletions

diff --git a/ChangeLog b/ChangeLog
index 9a382293..0ce3358a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2013-02-26  Jay Berkenbilt  <ejb@ql.org>
+
+	* Rewrite QUtil::int_to_string and QUtil::double_to_string to
+	remove internal length limits but to remain backward compatible
+	with the old versions for valid inputs.
+
 2013-02-23  Jay Berkenbilt  <ejb@ql.org>
 
 	* Bug fix: properly handle overridden compressed objects.  When
diff --git a/configure.ac b/configure.ac
index c423b086..06a9e207 100644
--- a/configure.ac
+++ b/configure.ac
@@ -68,24 +68,6 @@ AC_CHECK_FUNCS([fseeko64])
 AC_TYPE_UINT16_T
 AC_TYPE_UINT32_T
 
-AC_MSG_CHECKING(for whether printf supports %ll)
-AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
-#include <stdio.h>
-#include <string.h>
-#include <sys/types.h>
-int
-main()
-{
-    long long a = 160591605916059ll;
-    char t[50];
-    sprintf(t, "%lld", a);
-}
-]])],[qpdf_PRINTF_LL=yes],[qpdf_PRINTF_LL=no])
-AC_MSG_RESULT($qpdf_PRINTF_LL)
-if test "$qpdf_PRINTF_LL" = "yes"; then
-   AC_DEFINE([HAVE_PRINTF_LL], [1], [Whether printf supports %ll])
-fi
-
 AC_CHECK_FUNCS(random)
 
 # Check if LD supports linker scripts, and define conditional
diff --git a/libqpdf/QUtil.cc b/libqpdf/QUtil.cc
index 9549bfa1..967e832c 100644
--- a/libqpdf/QUtil.cc
+++ b/libqpdf/QUtil.cc
@@ -4,6 +4,9 @@
 #include <qpdf/QUtil.hh>
 #include <qpdf/PointerHolder.hh>
 
+#include <cmath>
+#include <iomanip>
+#include <sstream>
 #include <stdio.h>
 #include <errno.h>
 #include <ctype.h>
@@ -19,72 +22,41 @@
 #endif
 
 std::string
-QUtil::int_to_string(long long num, int fullpad)
+QUtil::int_to_string(long long num, int length)
 {
-    // This routine will need to be recompiled if an int can be longer than
-    // 49 digits.
-    char t[50];
-
-    // -2 or -1 to leave space for the possible negative sign and for NUL...
-    if (abs(fullpad) > sizeof(t) - ((num < 0)?2:1))
-    {
-	throw std::logic_error("Util::int_to_string has been called with "
-			       "a padding value greater than its internal "
-			       "limit");
-    }
-
-#ifdef HAVE_PRINTF_LL
-# define PRINTF_LL "ll"
-#else
-# define PRINTF_LL "l"
-#endif
-    if (fullpad)
+    // Backward compatibility -- this function used to use sprintf
+    // with %0*d, so we interpret length such that a negative value
+    // appends spaces and a positive value prepends zeroes.
+    std::ostringstream buf;
+    buf << num;
+    std::string result;
+    if ((length > 0) &&
+        (buf.str().length() < static_cast<size_t>(length)))
     {
-	sprintf(t, "%0*" PRINTF_LL "d", fullpad, num);
+	result.append(length - buf.str().length(), '0');
     }
-    else
+    result += buf.str();
+    if ((length < 0) && (buf.str().length() < static_cast<size_t>(-length)))
     {
-	sprintf(t, "%" PRINTF_LL "d", num);
+	result.append(-length - buf.str().length(), ' ');
     }
-#undef PRINTF_LL
-
-    return std::string(t);
+    return result;
 }
 
 std::string
 QUtil::double_to_string(double num, int decimal_places)
 {
-    // This routine will need to be recompiled if a double can be longer than
-    // 99 digits.
-    char t[100];
-
-    std::string lhs = int_to_string(static_cast<int>(num));
-
-    // lhs.length() gives us the length of the part on the right hand
-    // side of the dot + 1 for the dot + decimal_places: total size of
-    // the required string.  -1 on the sizeof side to allow for NUL at
-    // the end.
-    //
-    // If decimal_places <= 0, it is as if no precision was provided
-    // so trust the buffer is big enough.  The following test will
-    // always pass in those cases.
-    if (decimal_places + 1 + static_cast<int>(lhs.length()) >
-        static_cast<int>(sizeof(t)) - 1)
-    {
-	throw std::logic_error("Util::double_to_string has been called with "
-			       "a number and a decimal places specification "
-			       "that would break an internal limit");
-    }
-
-    if (decimal_places)
-    {
-	sprintf(t, "%.*f", decimal_places, num);
-    }
-    else
+    // Backward compatibility -- this code used to use sprintf and
+    // treated decimal_places <= 0 to mean to use the default, which
+    // was six decimal places.  Also sprintf with %*.f interprents the
+    // length as fixed point rather than significant figures.
+    if (decimal_places <= 0)
     {
-	sprintf(t, "%f", num);
+        decimal_places = 6;
     }
-    return std::string(t);
+    std::ostringstream buf;
+    buf << std::setprecision(decimal_places) << std::fixed << num;
+    return buf.str();
 }
 
 long long
diff --git a/libtests/qtest/qutil/qutil.out b/libtests/qtest/qutil/qutil.out
index a48e6266..c855e98e 100644
--- a/libtests/qtest/qutil/qutil.out
+++ b/libtests/qtest/qutil/qutil.out
@@ -4,11 +4,10 @@
 3.141590
 3.142
 1000.123000
-exception 1: Util::int_to_string has been called with a padding value greater than its internal limit
-exception 2: Util::int_to_string has been called with a padding value greater than its internal limit
-exception 3: Util::int_to_string has been called with a padding value greater than its internal limit
-exception 4: Util::double_to_string has been called with a number and a decimal places specification that would break an internal limit
-exception 5: Util::double_to_string has been called with a number and a decimal places specification that would break an internal limit
+0.12340
+0.00012
+0.12346
+0.00012
 one
 7
 compare okay
diff --git a/libtests/qutil.cc b/libtests/qutil.cc
index ad77f3ea..4e8c7362 100644
--- a/libtests/qutil.cc
+++ b/libtests/qutil.cc
@@ -19,58 +19,11 @@ void string_conversion_test()
 	      << QUtil::int_to_string(16059, -7) << std::endl
 	      << QUtil::double_to_string(3.14159) << std::endl
 	      << QUtil::double_to_string(3.14159, 3) << std::endl
-	      << QUtil::double_to_string(1000.123, -1024) << std::endl;
-
-    try
-    {
-	// int_to_string bounds error
-	std::cout << QUtil::int_to_string(1, 50) << std::endl;
-    }
-    catch (std::logic_error &e)
-    {
-	std::cout << "exception 1: " << e.what() << std::endl;
-    }
-
-    try
-    {
-	// QUtil::int_to_string bounds error
-	std::cout << QUtil::int_to_string(1, -50) << std::endl;
-    }
-    catch (std::logic_error& e)
-    {
-	std::cout << "exception 2: " << e.what() << std::endl;
-    }
-
-    try
-    {
-	// QUtil::int_to_string bounds error
-	std::cout << QUtil::int_to_string(-1, 49) << std::endl;
-    }
-    catch (std::logic_error& e)
-    {
-	std::cout << "exception 3: " << e.what() << std::endl;
-    }
-
-
-    try
-    {
-	// QUtil::double_to_string bounds error
-	std::cout << QUtil::double_to_string(3.14159, 1024) << std::endl;
-    }
-    catch (std::logic_error& e)
-    {
-	std::cout << "exception 4: " << e.what() << std::endl;
-    }
-
-    try
-    {
-	// QUtil::double_to_string bounds error
-	std::cout << QUtil::double_to_string(1000.0, 95) << std::endl;
-    }
-    catch (std::logic_error& e)
-    {
-	std::cout << "exception 5: " << e.what() << std::endl;
-    }
+	      << QUtil::double_to_string(1000.123, -1024) << std::endl
+              << QUtil::double_to_string(.1234, 5) << std::endl
+              << QUtil::double_to_string(.0001234, 5) << std::endl
+              << QUtil::double_to_string(.123456, 5) << std::endl
+              << QUtil::double_to_string(.000123456, 5) << std::endl;
 
     std::string embedded_null = "one";
     embedded_null += '\0';