Skip to content

Commit

Permalink
core: to_sstring_strintf(): always use %g(or %lg) format for floating…
Browse files Browse the repository at this point in the history
… point values

Current implementation of to_sstring_strintf() and to_sstring()
functions may cause memory overrun and trashing for floating
point values: to_sstring_sprintf() uses an internal buffer of
(3 * sizeof(value type) + 3) bytes for an output string but
this may not be enough for some floating point values,
e.g. for float values like 1e-23 or 1e+23, since
to_string(float) uses %f format.

%g and %lg printf formats on the other hand allow us to use integer
values induced upper estimate for an output string length
(3 * sizeof(value type) + 2) since it covers the longest
floating point values output for this format too:
   - 13 for a float (4 bytes value).
   - 14 for a double (8 bytes value).
   - 15 for a long double (10 bytes value).

This way we may significantly simplify the to_sstring_strintf() implementation
while ensuring that there won't be any memory trashing.

Signed-off-by: Vlad Zolotarov <[email protected]>
  • Loading branch information
Vlad Zolotarov authored and avikivity committed Dec 6, 2015
1 parent c6a3d3a commit 8c86079
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 7 deletions.
2 changes: 1 addition & 1 deletion apps/memcached/memcache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ class cache {

ss << "size: " << _cache.size() << "\n";
ss << "buckets: " << _cache.bucket_count() << "\n";
ss << "load: " << to_sstring_sprintf((double)_cache.size() / _cache.bucket_count(), "%.2lf") << "\n";
ss << "load: " << sprint("%.2lf", (double)_cache.size() / _cache.bucket_count()) << "\n";
ss << "max bucket occupancy: " << max_size << "\n";
ss << "bucket occupancy histogram:\n";

Expand Down
8 changes: 4 additions & 4 deletions core/sstring.hh
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ static String make_sstring(Args&&... args)
template <typename string_type, typename T>
inline
string_type to_sstring_sprintf(T value, const char* fmt) {
char tmp[sizeof(value) * 3 + 3];
char tmp[sizeof(value) * 3 + 2];
auto len = std::sprintf(tmp, fmt, value);
using char_type = typename string_type::value_type;
return string_type(reinterpret_cast<char_type*>(tmp), len);
Expand Down Expand Up @@ -636,19 +636,19 @@ string_type to_sstring(unsigned long long value) {
template <typename string_type = sstring>
inline
string_type to_sstring(float value) {
return to_sstring_sprintf<string_type>(value, "%f");
return to_sstring_sprintf<string_type>(value, "%g");
}

template <typename string_type = sstring>
inline
string_type to_sstring(double value) {
return to_sstring_sprintf<string_type>(value, "%f");
return to_sstring_sprintf<string_type>(value, "%g");
}

template <typename string_type = sstring>
inline
string_type to_sstring(long double value) {
return to_sstring_sprintf<string_type>(value, "%Lf");
return to_sstring_sprintf<string_type>(value, "%Lg");
}

template <typename string_type = sstring>
Expand Down
4 changes: 2 additions & 2 deletions tests/httpd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,15 @@ SEASTAR_TEST_CASE(test_formatter)
sstring str = "abc";
BOOST_REQUIRE_EQUAL(json::formatter::to_json(str), "\"abc\"");
float f = 1;
BOOST_REQUIRE_EQUAL(json::formatter::to_json(f), "1.000000");
BOOST_REQUIRE_EQUAL(json::formatter::to_json(f), "1");
f = 1.0/0.0;
BOOST_CHECK_THROW(json::formatter::to_json(f), std::out_of_range);
f = -1.0/0.0;
BOOST_CHECK_THROW(json::formatter::to_json(f), std::out_of_range);
f = 0.0/0.0;
BOOST_CHECK_THROW(json::formatter::to_json(f), std::invalid_argument);
double d = -1;
BOOST_REQUIRE_EQUAL(json::formatter::to_json(d), "-1.000000");
BOOST_REQUIRE_EQUAL(json::formatter::to_json(d), "-1");
d = 1.0/0.0;
BOOST_CHECK_THROW(json::formatter::to_json(d), std::out_of_range);
d = -1.0/0.0;
Expand Down

0 comments on commit 8c86079

Please sign in to comment.