Commit 8be827761347b7a0a4ce6e7bdfa6fd4585606b21
1 parent
ed19516a
Rewrite QUtil::int_to_string and QUtil::double_to_string
Make them safer by avoiding any internal limits and replacing sprintf with std::ostringstream.
Showing
5 changed files
with
41 additions
and
129 deletions
ChangeLog
| 1 | +2013-02-26 Jay Berkenbilt <ejb@ql.org> | |
| 2 | + | |
| 3 | + * Rewrite QUtil::int_to_string and QUtil::double_to_string to | |
| 4 | + remove internal length limits but to remain backward compatible | |
| 5 | + with the old versions for valid inputs. | |
| 6 | + | |
| 1 | 7 | 2013-02-23 Jay Berkenbilt <ejb@ql.org> |
| 2 | 8 | |
| 3 | 9 | * Bug fix: properly handle overridden compressed objects. When | ... | ... |
configure.ac
| ... | ... | @@ -68,24 +68,6 @@ AC_CHECK_FUNCS([fseeko64]) |
| 68 | 68 | AC_TYPE_UINT16_T |
| 69 | 69 | AC_TYPE_UINT32_T |
| 70 | 70 | |
| 71 | -AC_MSG_CHECKING(for whether printf supports %ll) | |
| 72 | -AC_COMPILE_IFELSE([AC_LANG_SOURCE([[ | |
| 73 | -#include <stdio.h> | |
| 74 | -#include <string.h> | |
| 75 | -#include <sys/types.h> | |
| 76 | -int | |
| 77 | -main() | |
| 78 | -{ | |
| 79 | - long long a = 160591605916059ll; | |
| 80 | - char t[50]; | |
| 81 | - sprintf(t, "%lld", a); | |
| 82 | -} | |
| 83 | -]])],[qpdf_PRINTF_LL=yes],[qpdf_PRINTF_LL=no]) | |
| 84 | -AC_MSG_RESULT($qpdf_PRINTF_LL) | |
| 85 | -if test "$qpdf_PRINTF_LL" = "yes"; then | |
| 86 | - AC_DEFINE([HAVE_PRINTF_LL], [1], [Whether printf supports %ll]) | |
| 87 | -fi | |
| 88 | - | |
| 89 | 71 | AC_CHECK_FUNCS(random) |
| 90 | 72 | |
| 91 | 73 | # Check if LD supports linker scripts, and define conditional | ... | ... |
libqpdf/QUtil.cc
| ... | ... | @@ -4,6 +4,9 @@ |
| 4 | 4 | #include <qpdf/QUtil.hh> |
| 5 | 5 | #include <qpdf/PointerHolder.hh> |
| 6 | 6 | |
| 7 | +#include <cmath> | |
| 8 | +#include <iomanip> | |
| 9 | +#include <sstream> | |
| 7 | 10 | #include <stdio.h> |
| 8 | 11 | #include <errno.h> |
| 9 | 12 | #include <ctype.h> |
| ... | ... | @@ -19,72 +22,41 @@ |
| 19 | 22 | #endif |
| 20 | 23 | |
| 21 | 24 | std::string |
| 22 | -QUtil::int_to_string(long long num, int fullpad) | |
| 25 | +QUtil::int_to_string(long long num, int length) | |
| 23 | 26 | { |
| 24 | - // This routine will need to be recompiled if an int can be longer than | |
| 25 | - // 49 digits. | |
| 26 | - char t[50]; | |
| 27 | - | |
| 28 | - // -2 or -1 to leave space for the possible negative sign and for NUL... | |
| 29 | - if (abs(fullpad) > sizeof(t) - ((num < 0)?2:1)) | |
| 30 | - { | |
| 31 | - throw std::logic_error("Util::int_to_string has been called with " | |
| 32 | - "a padding value greater than its internal " | |
| 33 | - "limit"); | |
| 34 | - } | |
| 35 | - | |
| 36 | -#ifdef HAVE_PRINTF_LL | |
| 37 | -# define PRINTF_LL "ll" | |
| 38 | -#else | |
| 39 | -# define PRINTF_LL "l" | |
| 40 | -#endif | |
| 41 | - if (fullpad) | |
| 27 | + // Backward compatibility -- this function used to use sprintf | |
| 28 | + // with %0*d, so we interpret length such that a negative value | |
| 29 | + // appends spaces and a positive value prepends zeroes. | |
| 30 | + std::ostringstream buf; | |
| 31 | + buf << num; | |
| 32 | + std::string result; | |
| 33 | + if ((length > 0) && | |
| 34 | + (buf.str().length() < static_cast<size_t>(length))) | |
| 42 | 35 | { |
| 43 | - sprintf(t, "%0*" PRINTF_LL "d", fullpad, num); | |
| 36 | + result.append(length - buf.str().length(), '0'); | |
| 44 | 37 | } |
| 45 | - else | |
| 38 | + result += buf.str(); | |
| 39 | + if ((length < 0) && (buf.str().length() < static_cast<size_t>(-length))) | |
| 46 | 40 | { |
| 47 | - sprintf(t, "%" PRINTF_LL "d", num); | |
| 41 | + result.append(-length - buf.str().length(), ' '); | |
| 48 | 42 | } |
| 49 | -#undef PRINTF_LL | |
| 50 | - | |
| 51 | - return std::string(t); | |
| 43 | + return result; | |
| 52 | 44 | } |
| 53 | 45 | |
| 54 | 46 | std::string |
| 55 | 47 | QUtil::double_to_string(double num, int decimal_places) |
| 56 | 48 | { |
| 57 | - // This routine will need to be recompiled if a double can be longer than | |
| 58 | - // 99 digits. | |
| 59 | - char t[100]; | |
| 60 | - | |
| 61 | - std::string lhs = int_to_string(static_cast<int>(num)); | |
| 62 | - | |
| 63 | - // lhs.length() gives us the length of the part on the right hand | |
| 64 | - // side of the dot + 1 for the dot + decimal_places: total size of | |
| 65 | - // the required string. -1 on the sizeof side to allow for NUL at | |
| 66 | - // the end. | |
| 67 | - // | |
| 68 | - // If decimal_places <= 0, it is as if no precision was provided | |
| 69 | - // so trust the buffer is big enough. The following test will | |
| 70 | - // always pass in those cases. | |
| 71 | - if (decimal_places + 1 + static_cast<int>(lhs.length()) > | |
| 72 | - static_cast<int>(sizeof(t)) - 1) | |
| 73 | - { | |
| 74 | - throw std::logic_error("Util::double_to_string has been called with " | |
| 75 | - "a number and a decimal places specification " | |
| 76 | - "that would break an internal limit"); | |
| 77 | - } | |
| 78 | - | |
| 79 | - if (decimal_places) | |
| 80 | - { | |
| 81 | - sprintf(t, "%.*f", decimal_places, num); | |
| 82 | - } | |
| 83 | - else | |
| 49 | + // Backward compatibility -- this code used to use sprintf and | |
| 50 | + // treated decimal_places <= 0 to mean to use the default, which | |
| 51 | + // was six decimal places. Also sprintf with %*.f interprents the | |
| 52 | + // length as fixed point rather than significant figures. | |
| 53 | + if (decimal_places <= 0) | |
| 84 | 54 | { |
| 85 | - sprintf(t, "%f", num); | |
| 55 | + decimal_places = 6; | |
| 86 | 56 | } |
| 87 | - return std::string(t); | |
| 57 | + std::ostringstream buf; | |
| 58 | + buf << std::setprecision(decimal_places) << std::fixed << num; | |
| 59 | + return buf.str(); | |
| 88 | 60 | } |
| 89 | 61 | |
| 90 | 62 | long long | ... | ... |
libtests/qtest/qutil/qutil.out
| ... | ... | @@ -4,11 +4,10 @@ |
| 4 | 4 | 3.141590 |
| 5 | 5 | 3.142 |
| 6 | 6 | 1000.123000 |
| 7 | -exception 1: Util::int_to_string has been called with a padding value greater than its internal limit | |
| 8 | -exception 2: Util::int_to_string has been called with a padding value greater than its internal limit | |
| 9 | -exception 3: Util::int_to_string has been called with a padding value greater than its internal limit | |
| 10 | -exception 4: Util::double_to_string has been called with a number and a decimal places specification that would break an internal limit | |
| 11 | -exception 5: Util::double_to_string has been called with a number and a decimal places specification that would break an internal limit | |
| 7 | +0.12340 | |
| 8 | +0.00012 | |
| 9 | +0.12346 | |
| 10 | +0.00012 | |
| 12 | 11 | one |
| 13 | 12 | 7 |
| 14 | 13 | compare okay | ... | ... |
libtests/qutil.cc
| ... | ... | @@ -19,58 +19,11 @@ void string_conversion_test() |
| 19 | 19 | << QUtil::int_to_string(16059, -7) << std::endl |
| 20 | 20 | << QUtil::double_to_string(3.14159) << std::endl |
| 21 | 21 | << QUtil::double_to_string(3.14159, 3) << std::endl |
| 22 | - << QUtil::double_to_string(1000.123, -1024) << std::endl; | |
| 23 | - | |
| 24 | - try | |
| 25 | - { | |
| 26 | - // int_to_string bounds error | |
| 27 | - std::cout << QUtil::int_to_string(1, 50) << std::endl; | |
| 28 | - } | |
| 29 | - catch (std::logic_error &e) | |
| 30 | - { | |
| 31 | - std::cout << "exception 1: " << e.what() << std::endl; | |
| 32 | - } | |
| 33 | - | |
| 34 | - try | |
| 35 | - { | |
| 36 | - // QUtil::int_to_string bounds error | |
| 37 | - std::cout << QUtil::int_to_string(1, -50) << std::endl; | |
| 38 | - } | |
| 39 | - catch (std::logic_error& e) | |
| 40 | - { | |
| 41 | - std::cout << "exception 2: " << e.what() << std::endl; | |
| 42 | - } | |
| 43 | - | |
| 44 | - try | |
| 45 | - { | |
| 46 | - // QUtil::int_to_string bounds error | |
| 47 | - std::cout << QUtil::int_to_string(-1, 49) << std::endl; | |
| 48 | - } | |
| 49 | - catch (std::logic_error& e) | |
| 50 | - { | |
| 51 | - std::cout << "exception 3: " << e.what() << std::endl; | |
| 52 | - } | |
| 53 | - | |
| 54 | - | |
| 55 | - try | |
| 56 | - { | |
| 57 | - // QUtil::double_to_string bounds error | |
| 58 | - std::cout << QUtil::double_to_string(3.14159, 1024) << std::endl; | |
| 59 | - } | |
| 60 | - catch (std::logic_error& e) | |
| 61 | - { | |
| 62 | - std::cout << "exception 4: " << e.what() << std::endl; | |
| 63 | - } | |
| 64 | - | |
| 65 | - try | |
| 66 | - { | |
| 67 | - // QUtil::double_to_string bounds error | |
| 68 | - std::cout << QUtil::double_to_string(1000.0, 95) << std::endl; | |
| 69 | - } | |
| 70 | - catch (std::logic_error& e) | |
| 71 | - { | |
| 72 | - std::cout << "exception 5: " << e.what() << std::endl; | |
| 73 | - } | |
| 22 | + << QUtil::double_to_string(1000.123, -1024) << std::endl | |
| 23 | + << QUtil::double_to_string(.1234, 5) << std::endl | |
| 24 | + << QUtil::double_to_string(.0001234, 5) << std::endl | |
| 25 | + << QUtil::double_to_string(.123456, 5) << std::endl | |
| 26 | + << QUtil::double_to_string(.000123456, 5) << std::endl; | |
| 74 | 27 | |
| 75 | 28 | std::string embedded_null = "one"; |
| 76 | 29 | embedded_null += '\0'; | ... | ... |