DPL: improve DataSpecUtils::describe API in case of buffers

Just like snprintf, it makes sense to return the size of the formatted
output.

Author: ktf

Framework/Core/include/Framework/DataSpecUtils.h

@@ -152,10 +152,10 @@ struct DataSpecUtils {
   static bool validate(OutputSpec const& output);
 
   /// Same as the other describe, but uses a buffer to reduce memory churn.
-  static void describe(char* buffer, size_t size, InputSpec const& spec);
+  static size_t describe(char* buffer, size_t size, InputSpec const& spec);
 
   /// Same as the other describe, but uses a buffer to reduce memory churn.
-  static void describe(char* buffer, size_t size, OutputSpec const& spec);
+  static size_t describe(char* buffer, size_t size, OutputSpec const& spec);
 
   /// If possible extract the ConcreteDataMatcher from an InputSpec. This
   /// can be done either if the InputSpec is defined in terms for a ConcreteDataMatcher

Framework/Core/src/DataSpecUtils.cxx

@@ -87,38 +87,30 @@ std::string DataSpecUtils::describe(OutputSpec const& spec)
                     spec.matcher);
 }
 
-void DataSpecUtils::describe(char* buffer, size_t size, InputSpec const& spec)
+size_t DataSpecUtils::describe(char* buffer, size_t size, InputSpec const& spec)
 {
   if (auto concrete = std::get_if<ConcreteDataMatcher>(&spec.matcher)) {
-    char origin[5];
-    origin[4] = 0;
-    char description[17];
-    description[16] = 0;
-    snprintf(buffer, size, "%s/%s/%" PRIu32, (strncpy(origin, concrete->origin.str, 4), origin),
-             (strncpy(description, concrete->description.str, 16), description), concrete->subSpec);
+    auto out = fmt::format_to_n(buffer, size, "{:.4}/{:.16}/{}", concrete->origin.str, concrete->description.str, concrete->subSpec);
+    return std::min(out.size, size);
   } else if (auto matcher = std::get_if<DataDescriptorMatcher>(&spec.matcher)) {
     std::ostringstream ss;
     ss << "<matcher query: " << *matcher << ">";
-    strncpy(buffer, ss.str().c_str(), size - 1);
-  } else {
-    throw runtime_error("Unsupported InputSpec");
+    auto out = fmt::format_to_n(buffer, size, "{}", ss.str());
+    return std::min(out.size, size);
   }
+  throw runtime_error("Unsupported InputSpec");
 }
 
-void DataSpecUtils::describe(char* buffer, size_t size, OutputSpec const& spec)
+size_t DataSpecUtils::describe(char* buffer, size_t size, OutputSpec const& spec)
 {
   if (auto concrete = std::get_if<ConcreteDataMatcher>(&spec.matcher)) {
-    char origin[5];
-    origin[4] = 0;
-    char description[17];
-    description[16] = 0;
-    snprintf(buffer, size, "%s/%s/%" PRIu32, (strncpy(origin, concrete->origin.str, 4), origin),
-             (strncpy(description, concrete->description.str, 16), description), concrete->subSpec);
+    auto out = fmt::format_to_n(buffer, size, "{:.4}/{:.16}/{}", concrete->origin.str, concrete->description.str, concrete->subSpec);
+    return std::min(out.size, size);
   } else if (auto concrete = std::get_if<ConcreteDataTypeMatcher>(&spec.matcher)) {
-    fmt::format_to(buffer, "<matcher query: {}/{}>", concrete->origin, concrete->description);
-  } else {
-    throw runtime_error("Unsupported OutputSpec");
+    auto out = fmt::format_to_n(buffer, size, "<matcher query: {}/{}>", concrete->origin, concrete->description);
+    return std::min(out.size, size);
   }
+  throw runtime_error("Unsupported OutputSpec");
 }
 
 std::string DataSpecUtils::label(InputSpec const& spec)

Framework/Core/test/unittest_DataSpecUtils.cxx

@@ -42,6 +42,7 @@ TEST_CASE("ConcreteData")
     CHECK(std::string(concrete.description.as<std::string>()) == "FOOO");
     CHECK(concrete.subSpec == 1);
     CHECK(DataSpecUtils::describe(spec) == "TEST/FOOO/1");
+    CHECK(DataSpecUtils::describe(spec) == "TEST/FOOO/1");
     CHECK(*DataSpecUtils::getOptionalSubSpec(spec) == 1);
 
     ConcreteDataTypeMatcher dataType = DataSpecUtils::asConcreteDataTypeMatcher(spec);
@@ -59,6 +60,43 @@ TEST_CASE("ConcreteData")
   }
 }
 
+TEST_CASE("DescribeUsingBuffer")
+{
+  o2::framework::clean_all_runtime_errors();
+  OutputSpec spec{
+    "TEST",
+    "FOOO",
+    1,
+    Lifetime::Timeframe};
+
+  InputSpec inputSpec{
+    "binding",
+    "TEST",
+    "FOOO",
+    1,
+    Lifetime::Timeframe};
+
+  REQUIRE(DataSpecUtils::validate(inputSpec));
+
+  {
+    char buffer[1024];
+
+    ConcreteDataMatcher concrete = DataSpecUtils::asConcreteDataMatcher(spec);
+    CHECK(std::string(concrete.origin.as<std::string>()) == "TEST");
+    CHECK(std::string(concrete.description.as<std::string>()) == "FOOO");
+    CHECK(concrete.subSpec == 1);
+    auto size = DataSpecUtils::describe(buffer, 1024, spec);
+    CHECK(std::string_view(buffer, size) == "TEST/FOOO/1");
+    size = DataSpecUtils::describe(buffer, 1024, spec);
+    CHECK(std::string_view(buffer, size) == "TEST/FOOO/1");
+    CHECK(*DataSpecUtils::getOptionalSubSpec(spec) == 1);
+
+    char buffer2[1024];
+    size = DataSpecUtils::describe(buffer2, 5, spec);
+    CHECK(std::string_view(buffer2, size) == "TEST/");
+  }
+}
+
 TEST_CASE("WithWildCards")
 {
   OutputSpec spec{
@@ -78,6 +116,21 @@ TEST_CASE("WithWildCards")
   CHECK(DataSpecUtils::getOptionalSubSpec(spec) == std::nullopt);
 }
 
+TEST_CASE("WithWildCardsBuffer")
+{
+  char buffer[1024];
+  OutputSpec spec{
+    {"TEST", "FOOO"},
+    Lifetime::Timeframe};
+
+  auto size = DataSpecUtils::describe(buffer, 1024, spec);
+  CHECK(std::string_view(buffer, size) == "<matcher query: TEST/FOOO>");
+
+  char buffer2[1024];
+  size = DataSpecUtils::describe(buffer2, 5, spec);
+  CHECK( std::string_view(buffer2, size) == "<matc");
+}
+
 TEST_CASE("MatchingInputs")
 {
   OutputSpec fullySpecified{