Formatters: improve precision of double numbers

Default precision is not enough for at least JSON
formatter. So some information loss possible. This
fix makes numbers long enough.

Signed-off-by: Коренберг Марк <socketpair@gmail.com>

diff --git a/src/common/Formatter.cc b/src/common/Formatter.cc
index 8f199ef7a7b2950045d6050e5dd951229f26ccad..f2498c9329d8342e2c06521f911780aa4ed1ffe7 100644 (file)
--- a/src/common/Formatter.cc
+++ b/src/common/Formatter.cc
@@ -19,6 +19,7 @@
 #include "include/buffer.h"
 
 #include <set>
+#include <limits>
 #include <boost/format.hpp>
 
 // -----------------------
@@ -262,6 +263,7 @@ template <class T>
 void JSONFormatter::add_value(const char *name, T val)
 {
   std::stringstream ss;
+  ss.precision(std::numeric_limits<T>::max_digits10);
   ss << val;
   add_value(name, ss.str(), false);
 }
@@ -291,9 +293,7 @@ void JSONFormatter::dump_int(const char *name, int64_t s)
 
 void JSONFormatter::dump_float(const char *name, double d)
 {
-  char foo[30];
-  snprintf(foo, sizeof(foo), "%lf", d);
-  add_value(name, foo, false);
+  add_value(name, d);
 }
 
 void JSONFormatter::dump_string(const char *name, std::string_view s)
@@ -426,40 +426,33 @@ void XMLFormatter::close_section()
     m_ss << "\n";
 }
 
-void XMLFormatter::dump_unsigned(const char *name, uint64_t u)
+template <class T>
+void XMLFormatter::add_value(const char *name, T val)
 {
   std::string e(name);
   std::transform(e.begin(), e.end(), e.begin(),
       [this](char c) { return this->to_lower_underscore(c); });
 
   print_spaces();
-  m_ss << "<" << e << ">" << u << "</" << e << ">";
+  m_ss.precision(std::numeric_limits<T>::max_digits10);
+  m_ss << "<" << e << ">" << val << "</" << e << ">";
   if (m_pretty)
     m_ss << "\n";
 }
 
-void XMLFormatter::dump_int(const char *name, int64_t u)
+void XMLFormatter::dump_unsigned(const char *name, uint64_t u)
 {
-  std::string e(name);
-  std::transform(e.begin(), e.end(), e.begin(),
-      [this](char c) { return this->to_lower_underscore(c); });
+  add_value(name, u);
+}
 
-  print_spaces();
-  m_ss << "<" << e << ">" << u << "</" << e << ">";
-  if (m_pretty)
-    m_ss << "\n";
+void XMLFormatter::dump_int(const char *name, int64_t s)
+{
+  add_value(name, s);
 }
 
 void XMLFormatter::dump_float(const char *name, double d)
 {
-  std::string e(name);
-  std::transform(e.begin(), e.end(), e.begin(),
-      [this](char c) { return this->to_lower_underscore(c); });
-
-  print_spaces();
-  m_ss << "<" << e << ">" << d << "</" << e << ">";
-  if (m_pretty)
-    m_ss << "\n";
+  add_value(name, d);
 }
 
 void XMLFormatter::dump_string(const char *name, std::string_view s)
@@ -817,35 +810,31 @@ std::string TableFormatter::get_section_name(const char* name)
   }
 }
 
-void TableFormatter::dump_unsigned(const char *name, uint64_t u)
-{
+template <class T>
+void TableFormatter::add_value(const char *name, T val) {
   finish_pending_string();
   size_t i = m_vec_index(name);
-  m_ss << u;
+  m_ss.precision(std::numeric_limits<double>::max_digits10);
+  m_ss << val;
+
   m_vec[i].push_back(std::make_pair(get_section_name(name), m_ss.str()));
   m_ss.clear();
   m_ss.str("");
 }
 
-void TableFormatter::dump_int(const char *name, int64_t u)
+void TableFormatter::dump_unsigned(const char *name, uint64_t u)
 {
-  finish_pending_string();
-  size_t i = m_vec_index(name);
-  m_ss << u;
-  m_vec[i].push_back(std::make_pair(get_section_name(name), m_ss.str()));
-  m_ss.clear();
-  m_ss.str("");
+  add_value(name, u);
 }
 
-void TableFormatter::dump_float(const char *name, double d)
+void TableFormatter::dump_int(const char *name, int64_t s)
 {
-  finish_pending_string();
-  size_t i = m_vec_index(name);
-  m_ss << d;
+  add_value(name, s);
+}
 
-  m_vec[i].push_back(std::make_pair(get_section_name(name), m_ss.str()));
-  m_ss.clear();
-  m_ss.str("");
+void TableFormatter::dump_float(const char *name, double d)
+{
+  add_value(name, d);
 }
 
 void TableFormatter::dump_string(const char *name, std::string_view s)

diff --git a/src/common/Formatter.h b/src/common/Formatter.h
index 9d984c11f87512e4bc4ddcafd9b7d06339e5e65d..b20a2a5fe97bad29aa7c94741ae8eec804886ccf 100644 (file)
--- a/src/common/Formatter.h
+++ b/src/common/Formatter.h
@@ -139,7 +139,7 @@ namespace ceph {
     void open_object_section_in_ns(const char *name, const char *ns) override;
     void close_section() override;
     void dump_unsigned(const char *name, uint64_t u) override;
-    void dump_int(const char *name, int64_t u) override;
+    void dump_int(const char *name, int64_t s) override;
     void dump_float(const char *name, double d) override;
     void dump_string(const char *name, std::string_view s) override;
     std::ostream& dump_stream(const char *name) override;
@@ -209,7 +209,7 @@ namespace ceph {
     void open_object_section_in_ns(const char *name, const char *ns) override;
     void close_section() override;
     void dump_unsigned(const char *name, uint64_t u) override;
-    void dump_int(const char *name, int64_t u) override;
+    void dump_int(const char *name, int64_t s) override;
     void dump_float(const char *name, double d) override;
     void dump_string(const char *name, std::string_view s) override;
     std::ostream& dump_stream(const char *name) override;
@@ -237,6 +237,9 @@ namespace ceph {
     std::string m_pending_string_name;
     bool m_header_done;
     bool m_line_break_enabled = false;
+  private:
+    template <class T>
+    void add_value(const char *name, T val);
   };
 
   class TableFormatter : public Formatter {
@@ -260,7 +263,7 @@ namespace ceph {
 
     void close_section() override;
     void dump_unsigned(const char *name, uint64_t u) override;
-    void dump_int(const char *name, int64_t u) override;
+    void dump_int(const char *name, int64_t s) override;
     void dump_float(const char *name, double d) override;
     void dump_string(const char *name, std::string_view s) override;
     void dump_format_va(const char *name, const char *ns, bool quoted, const char *fmt, va_list ap) override;
@@ -272,6 +275,8 @@ namespace ceph {
     void get_attrs_str(const FormatterAttrs *attrs, std::string& attrs_str);
 
   private:
+    template <class T>
+    void add_value(const char *name, T val);
     void open_section_in_ns(const char *name, const char *ns, const FormatterAttrs *attrs);
     std::vector< std::vector<std::pair<std::string, std::string> > > m_vec;
     std::stringstream m_ss;

diff --git a/src/test/cli/crushtool/choose-args.t b/src/test/cli/crushtool/choose-args.t
index 1172aefb3ff2165c0e86881fdfd77edff0764419..db9c50bed245617431fe113b14a733f57a9f696e 100644 (file)
--- a/src/test/cli/crushtool/choose-args.t
+++ b/src/test/cli/crushtool/choose-args.t
@@ -194,14 +194,14 @@
                   "bucket_id": -3,
                   "weight_set": [
                       [
-                          1.000000,
-                          2.000000,
-                          5.000000
+                          1,
+                          2,
+                          5
                       ],
                       [
-                          3.000000,
-                          2.000000,
-                          5.000000
+                          3,
+                          2,
+                          5
                       ]
                   ],
                   "ids": [
@@ -216,10 +216,10 @@
                   "bucket_id": -2,
                   "weight_set": [
                       [
-                          1.000000
+                          1
                       ],
                       [
-                          3.000000
+                          3
                       ]
                   ]
               }
@@ -243,10 +243,10 @@
                   "bucket_id": -2,
                   "weight_set": [
                       [
-                          1.000000
+                          1
                       ],
                       [
-                          3.000000
+                          3
                       ]
                   ]
               },
@@ -254,14 +254,14 @@
                   "bucket_id": -3,
                   "weight_set": [
                       [
-                          1.000000,
-                          2.000000,
-                          5.000000
+                          1,
+                          2,
+                          5
                       ],
                       [
-                          3.000000,
-                          2.000000,
-                          5.000000
+                          3,
+                          2,
+                          5
                       ]
                   ],
                   "ids": [

diff --git a/src/test/cli/osdmaptool/tree.t b/src/test/cli/osdmaptool/tree.t
index b0f06d7f6c2e5e873d6fcc304149df692b1bd938..394a4f38436a944ff8c4334b1d37e81a5513a393 100644 (file)
--- a/src/test/cli/osdmaptool/tree.t
+++ b/src/test/cli/osdmaptool/tree.t
@@ -52,39 +52,39 @@
               "name": "osd.0",
               "type": "osd",
               "type_id": 0,
-              "crush_weight": 1.000000,
+              "crush_weight": 1,
               "depth": 3,
               "pool_weights": {},
               "exists": 0,
               "status": "down",
-              "reweight": 0.000000,
-              "primary_affinity": 1.000000
+              "reweight": 0,
+              "primary_affinity": 1
           },
           {
               "id": 1,
               "name": "osd.1",
               "type": "osd",
               "type_id": 0,
-              "crush_weight": 1.000000,
+              "crush_weight": 1,
               "depth": 3,
               "pool_weights": {},
               "exists": 0,
               "status": "down",
-              "reweight": 0.000000,
-              "primary_affinity": 1.000000
+              "reweight": 0,
+              "primary_affinity": 1
           },
           {
               "id": 2,
               "name": "osd.2",
               "type": "osd",
               "type_id": 0,
-              "crush_weight": 1.000000,
+              "crush_weight": 1,
               "depth": 3,
               "pool_weights": {},
               "exists": 0,
               "status": "down",
-              "reweight": 0.000000,
-              "primary_affinity": 1.000000
+              "reweight": 0,
+              "primary_affinity": 1
           }
       ],
       "stray": []

diff --git a/src/test/common/test_tableformatter.cc b/src/test/common/test_tableformatter.cc
index ff99e84374e70901e2c8e2192dda31c82231686c..b152014a2b5e3d67ea5ad64fd6d9ddaa86c3920a 100644 (file)
--- a/src/test/common/test_tableformatter.cc
+++ b/src/test/common/test_tableformatter.cc
@@ -25,6 +25,22 @@ TEST(tableformatter, singleline)
   EXPECT_EQ(cmp, sout.str());
 }
 
+TEST(tableformatter, longfloat)
+{
+  std::stringstream sout;
+  TableFormatter formatter;
+  formatter.dump_float("float", 1.0 / 7);
+  formatter.flush(sout);
+
+  std::string cmp = ""
+    "+----------------------+\n"
+    "| float                |\n"
+    "+----------------------+\n"
+    "| 0.14285714285714285  |\n"
+    "+----------------------+\n";
+  EXPECT_EQ(cmp, sout.str());
+}
+
 TEST(tableformatter, multiline)
 {
   std::stringstream sout;

diff --git a/src/test/formatter.cc b/src/test/formatter.cc
index da8cc937e71246ad74f45c8e8073ee970430e5a7..1e78a18a006cbb19b72b2aeca515207fd90a2531 100644 (file)
--- a/src/test/formatter.cc
+++ b/src/test/formatter.cc
@@ -47,10 +47,21 @@ TEST(JsonFormatter, Simple2) {
   fmt.close_section();
   fmt.flush(oss);
   ASSERT_EQ(oss.str(), "{\"bar\":{\"int\":263882790666240,\
-\"unsigned\":9223372036854775809,\"float\":1.234000},\
+\"unsigned\":9223372036854775809,\"float\":1.234},\
 \"string\":\"str\"}");
 }
 
+TEST(JsonFormatter, CunningFloats) {
+  ostringstream oss;
+  JSONFormatter fmt(false);
+  fmt.open_object_section("foo");
+  fmt.dump_float("long", 1.0 / 7);
+  fmt.dump_float("big", 12345678901234567890.0);
+  fmt.close_section();
+  fmt.flush(oss);
+  ASSERT_EQ(oss.str(), "{\"long\":0.14285714285714285,\"big\":1.2345678901234567e+19}");
+}
+
 TEST(JsonFormatter, Empty) {
   ostringstream oss;
   JSONFormatter fmt(false);
@@ -122,10 +133,10 @@ TEST(XmlFormatter, DumpStream3) {
 
   fmt.open_array_section("foo");
   fmt.dump_stream("blah") << "hithere";
-  fmt.dump_float("pi", 3.14);
+  fmt.dump_float("pi", 0.128);
   fmt.close_section();
   fmt.flush(oss);
-  ASSERT_EQ(oss.str(), "<foo><blah>hithere</blah><pi>3.14</pi></foo>");
+  ASSERT_EQ(oss.str(), "<foo><blah>hithere</blah><pi>0.128</pi></foo>");
 }
 
 TEST(XmlFormatter, DTD) {
@@ -135,11 +146,11 @@ TEST(XmlFormatter, DTD) {
   fmt.output_header();
   fmt.open_array_section("foo");
   fmt.dump_stream("blah") << "hithere";
-  fmt.dump_float("pi", 3.14);
+  fmt.dump_float("pi", 0.128);
   fmt.close_section();
   fmt.flush(oss);
   ASSERT_EQ(oss.str(), "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
-    "<foo><blah>hithere</blah><pi>3.14</pi></foo>");
+    "<foo><blah>hithere</blah><pi>0.128</pi></foo>");
 }
 
 TEST(XmlFormatter, Clear) {
@@ -149,11 +160,11 @@ TEST(XmlFormatter, Clear) {
   fmt.output_header();
   fmt.open_array_section("foo");
   fmt.dump_stream("blah") << "hithere";
-  fmt.dump_float("pi", 3.14);
+  fmt.dump_float("pi", 0.128);
   fmt.close_section();
   fmt.flush(oss);
   ASSERT_EQ(oss.str(), "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
-    "<foo><blah>hithere</blah><pi>3.14</pi></foo>");
+    "<foo><blah>hithere</blah><pi>0.128</pi></foo>");
 
   ostringstream oss2;
   fmt.flush(oss2);
@@ -173,12 +184,12 @@ TEST(XmlFormatter, NamespaceTest) {
   fmt.open_array_section_in_ns("foo",
                           "http://s3.amazonaws.com/doc/2006-03-01/");
   fmt.dump_stream("blah") << "hithere";
-  fmt.dump_float("pi", 3.14);
+  fmt.dump_float("pi", 0.128);
   fmt.close_section();
   fmt.flush(oss);
   ASSERT_EQ(oss.str(), "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
     "<foo xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
-    "<blah>hithere</blah><pi>3.14</pi></foo>");
+    "<blah>hithere</blah><pi>0.128</pi></foo>");
 }
 
 TEST(XmlFormatter, DumpFormatNameSpaceTest) {
@@ -264,10 +275,10 @@ TEST(HtmlFormatter, DumpStream3) {
 
   fmt.open_array_section("foo");
   fmt.dump_stream("blah") << "hithere";
-  fmt.dump_float("pi", 3.14);
+  fmt.dump_float("pi", 0.128);
   fmt.close_section();
   fmt.flush(oss);
-  ASSERT_EQ(oss.str(), "<foo><li>blah: hithere</li><li>pi: 3.14</li></foo>");
+  ASSERT_EQ(oss.str(), "<foo><li>blah: hithere</li><li>pi: 0.128</li></foo>");
 }
 
 TEST(HtmlFormatter, DTD) {
@@ -277,11 +288,11 @@ TEST(HtmlFormatter, DTD) {
   fmt.write_raw_data(HTMLFormatter::XML_1_DTD);
   fmt.open_array_section("foo");
   fmt.dump_stream("blah") << "hithere";
-  fmt.dump_float("pi", 3.14);
+  fmt.dump_float("pi", 0.128);
   fmt.close_section();
   fmt.flush(oss);
   ASSERT_EQ(oss.str(), "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
-    "<foo><li>blah: hithere</li><li>pi: 3.14</li></foo>");
+    "<foo><li>blah: hithere</li><li>pi: 0.128</li></foo>");
 }
 
 TEST(HtmlFormatter, Clear) {
@@ -291,11 +302,11 @@ TEST(HtmlFormatter, Clear) {
   fmt.write_raw_data(HTMLFormatter::XML_1_DTD);
   fmt.open_array_section("foo");
   fmt.dump_stream("blah") << "hithere";
-  fmt.dump_float("pi", 3.14);
+  fmt.dump_float("pi", 0.128);
   fmt.close_section();
   fmt.flush(oss);
   ASSERT_EQ(oss.str(), "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
-    "<foo><li>blah: hithere</li><li>pi: 3.14</li></foo>");
+    "<foo><li>blah: hithere</li><li>pi: 0.128</li></foo>");
 
   ostringstream oss2;
   fmt.flush(oss2);
@@ -315,12 +326,12 @@ TEST(HtmlFormatter, NamespaceTest) {
   fmt.open_array_section_in_ns("foo",
                           "http://s3.amazonaws.com/doc/2006-03-01/");
   fmt.dump_stream("blah") << "hithere";
-  fmt.dump_float("pi", 3.14);
+  fmt.dump_float("pi", 0.128);
   fmt.close_section();
   fmt.flush(oss);
   ASSERT_EQ(oss.str(), "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
     "<foo xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">"
-    "<li>blah: hithere</li><li>pi: 3.14</li></foo>");
+    "<li>blah: hithere</li><li>pi: 0.128</li></foo>");
 }
 
 TEST(HtmlFormatter, DumpFormatNameSpaceTest) {