Merge pull request #49624 from ronen-fr/wip-rf-unique-formatter

common: extend Formatter API with a unique_ptr-returning static function

src/common/Formatter.h

@@ -8,6 +8,7 @@
 
 #include <deque>
 #include <list>
+#include <memory>
 #include <vector>
 #include <stdarg.h>
 #include <sstream>
@@ -62,6 +63,12 @@ namespace ceph {
     static Formatter *create(std::string_view type) {
       return create(type, "json-pretty", "");
     }
+    template <typename... Params>
+    static std::unique_ptr<Formatter> create_unique(Params &&...params)
+    {
+      return std::unique_ptr<Formatter>(
+	  Formatter::create(std::forward<Params>(params)...));
+    }
 
     Formatter();
     virtual ~Formatter();

src/osd/PrimaryLogPG.cc

@@ -1001,8 +1001,7 @@ void PrimaryLogPG::do_command(
 {
   string format;
   cmd_getval(cmdmap, "format", format);
-  std::unique_ptr<Formatter> f(Formatter::create(
-				 format, "json-pretty", "json-pretty"));
+  auto f(Formatter::create_unique(format, "json-pretty", "json-pretty"));
   int ret = 0;
   stringstream ss;   // stderr error message stream
   bufferlist outbl;  // if empty at end, we'll dump formatter as output
@@ -15071,12 +15070,11 @@ bool PrimaryLogPG::agent_maybe_evict(ObjectContextRef& obc, bool after_flush)
 	     << ", evict_effort " << agent_state->evict_effort
 	     << dendl;
     dout(30) << "agent_state:\n";
-    Formatter *f = Formatter::create("");
+    auto f = Formatter::create_unique("");
     f->open_object_section("agent_state");
-    agent_state->dump(f);
+    agent_state->dump(f.get());
     f->close_section();
     f->flush(*_dout);
-    delete f;
     *_dout << dendl;
 
     if (1000000 - temp_upper >= agent_state->evict_effort)

src/test/common/get_command_descriptions.cc

@@ -46,10 +46,9 @@ static void usage(ostream &out)
 static void json_print(const std::vector<MonCommand> &mon_commands)
 {
   bufferlist rdata;
-  Formatter *f = Formatter::create("json");
-  Monitor::format_command_descriptions(mon_commands, f,
+  auto f = Formatter::create_unique("json");
+  Monitor::format_command_descriptions(mon_commands, f.get(),
                                        CEPH_FEATURES_ALL, &rdata);
-  delete f;
   string data(rdata.c_str(), rdata.length());
   cout << data << std::endl;
 }

src/test/common/test_context.cc

@@ -43,7 +43,7 @@ TEST(CephContext, do_command)
   {
     stringstream ss;
     bufferlist out;
-    std::unique_ptr<Formatter> f(Formatter::create("xml", "xml"));
+    std::unique_ptr<Formatter> f{Formatter::create_unique("xml", "xml")};
     cct->do_command("config get", cmdmap, f.get(), ss, &out);
     f->flush(out);
     string s(out.c_str(), out.length());
@@ -54,7 +54,7 @@ TEST(CephContext, do_command)
     stringstream ss;
     bufferlist out;
     cmdmap_t bad_cmdmap; // no 'var' field
-    std::unique_ptr<Formatter> f(Formatter::create("xml", "xml"));
+    std::unique_ptr<Formatter> f{Formatter::create_unique("xml", "xml")};
     int r = cct->do_command("config get", bad_cmdmap, f.get(), ss, &out);
     if (r >= 0) {
       f->flush(out);
@@ -69,7 +69,7 @@ TEST(CephContext, do_command)
     bufferlist out;
     cmdmap_t bad_cmdmap;
     bad_cmdmap["var"] = string("doesnotexist123");
-    std::unique_ptr<Formatter> f(Formatter::create("xml", "xml"));
+    std::unique_ptr<Formatter> f{Formatter::create_unique("xml", "xml")};
     int r = cct->do_command("config help", bad_cmdmap, f.get(), ss, &out);
     if (r >= 0) {
       f->flush(out);
@@ -83,7 +83,7 @@ TEST(CephContext, do_command)
   {
     stringstream ss;
     bufferlist out;
-    std::unique_ptr<Formatter> f(Formatter::create("xml", "xml"));
+    std::unique_ptr<Formatter> f{Formatter::create_unique("xml", "xml")};
     cct->do_command("config diff get", cmdmap, f.get(), ss, &out);
     f->flush(out);
     string s(out.c_str(), out.length());

src/test/common/test_sloppy_crc_map.cc

@@ -11,12 +11,11 @@ using namespace std;
 
 void dump(const SloppyCRCMap& scm)
 {
-  Formatter *f = Formatter::create("json-pretty");
+  auto f = Formatter::create_unique("json-pretty");
   f->open_object_section("map");
-  scm.dump(f);
+  scm.dump(f.get());
   f->close_section();
   f->flush(cout);
-  delete f;
 }
 
 TEST(SloppyCRCMap, basic) {

src/test/crush/CrushWrapper.cc

@@ -901,13 +901,12 @@ TEST_F(CrushWrapperTest, dump_rules) {
 
   // no rule by default
   {
-    Formatter *f = Formatter::create("json-pretty");
+    auto f = Formatter::create_unique("json-pretty");
     f->open_array_section("rules");
-    c->dump_rules(f);
+    c->dump_rules(f.get());
     f->close_section();
     stringstream ss;
     f->flush(ss);
-    delete f;
     EXPECT_EQ("[]\n", ss.str());
   }
 
@@ -917,20 +916,18 @@ TEST_F(CrushWrapperTest, dump_rules) {
   EXPECT_EQ(0, rule);
 
   {
-    Formatter *f = Formatter::create("xml");
-    c->dump_rules(f);
+    auto f = Formatter::create_unique("xml");
+    c->dump_rules(f.get());
     stringstream ss;
     f->flush(ss);
-    delete f;
     EXPECT_EQ((unsigned)0, ss.str().find("<rule><rule_id>0</rule_id><rule_name>NAME</rule_name>"));
   }
 
   {
-    Formatter *f = Formatter::create("xml");
-    c->dump_rule(rule, f);
+    auto f = Formatter::create_unique("xml");
+    c->dump_rule(rule, f.get());
     stringstream ss;
     f->flush(ss);
-    delete f;
     EXPECT_EQ((unsigned)0, ss.str().find("<rule><rule_id>0</rule_id><rule_name>NAME</rule_name>"));
     EXPECT_NE(string::npos,
 	      ss.str().find("<item_name>default</item_name></step>"));

src/test/objectstore/test_bluestore_types.cc

@@ -56,13 +56,12 @@ TEST(bluestore, sizeof) {
 void dump_mempools()
 {
   ostringstream ostr;
-  Formatter* f = Formatter::create("json-pretty", "json-pretty", "json-pretty");
+  auto f = Formatter::create_unique("json-pretty", "json-pretty", "json-pretty");
   ostr << "Mempools: ";
   f->open_object_section("mempools");
-  mempool::dump(f);
+  mempool::dump(f.get());
   f->close_section();
   f->flush(ostr);
-  delete f;
   cout << ostr.str() << std::endl;
 }
 /*void get_mempool_stats(uint64_t* total_bytes, uint64_t* total_items)