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

@@ -18,11 +18,12 @@
 
 #include <optional>
 
-namespace o2
-{
-namespace framework
+namespace o2::framework
 {
 
+template <typename T>
+concept HasMatcher =  requires (T& t) { t.matcher; };
+
 struct DataSpecUtils {
   /// @return true if a given InputSpec @a spec matches with a @a target ConcreteDataMatcher
   static bool match(InputSpec const& spec, ConcreteDataMatcher const& target);
@@ -152,10 +153,8 @@ 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);
-
-  /// Same as the other describe, but uses a buffer to reduce memory churn.
-  static void describe(char* buffer, size_t size, OutputSpec const& spec);
+  template <HasMatcher T>
+  static size_t describe(char* buffer, size_t size, T 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
@@ -250,6 +249,6 @@ struct DataSpecUtils {
   static void updateOutputList(std::vector<OutputSpec>& list, OutputSpec&& input);
 };
 
-} // namespace framework
-} // namespace o2
+} // namespace o2::framework
+
 #endif // FRAMEWORK_DATASPECUTILS_H

Framework/Core/src/DataSpecUtils.cxx

@@ -15,11 +15,13 @@
 #include "Framework/RuntimeError.h"
 #include "Headers/DataHeaderHelpers.h"
 
+#include <fmt/base.h>
 #include <fmt/format.h>
 #include <sstream>
 #include <cstring>
 #include <cinttypes>
 #include <regex>
+#include <stdexcept>
 
 namespace o2::framework
 {
@@ -87,39 +89,29 @@ std::string DataSpecUtils::describe(OutputSpec const& spec)
                     spec.matcher);
 }
 
-void DataSpecUtils::describe(char* buffer, size_t size, InputSpec const& spec)
+template <HasMatcher T>
+size_t DataSpecUtils::describe(char* buffer, size_t size, T 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);
-  } 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 result = std::visit(overloaded{
+    [buffer, size](ConcreteDataMatcher const& concrete) -> fmt::format_to_n_result<char*> {
+      return fmt::format_to_n(buffer, size - 1, "{:.4}/{:.16}/{}", concrete.origin.str, concrete.description.str, concrete.subSpec);
+    },
+    [buffer, size](ConcreteDataTypeMatcher const& concrete) -> fmt::format_to_n_result<char*> {
+      return fmt::format_to_n(buffer, size - 1, "<matcher query: {}/{}>", concrete.origin, concrete.description);
+    },
+    [buffer, size](DataDescriptorMatcher const& matcher) -> fmt::format_to_n_result<char*> {
+      std::ostringstream ss;
+      ss << "<matcher query: " << matcher << ">";
+      return fmt::format_to_n(buffer, size - 1, "{}", ss.str());
+    },
+    [](...) -> fmt::format_to_n_result<char*> { throw std::runtime_error("Unsupported Input / Output Spec"); }
+  }, spec.matcher);
+  *result.out = '\0';
+  return result.out - buffer;
 }
 
-void 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);
-  } 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");
-  }
-}
+template size_t DataSpecUtils::describe(char* buffer, size_t size, InputSpec const& spec);
+template size_t DataSpecUtils::describe(char* buffer, size_t size, OutputSpec const& spec);
 
 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,44 @@ 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);
+    // We always nullterminate the string
+    CHECK(std::string_view(buffer2, size) == "TEST");
+  }
+}
+
 TEST_CASE("WithWildCards")
 {
   OutputSpec spec{
@@ -78,6 +117,22 @@ 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);
+  // We always null terminate the buffer.
+  CHECK( std::string_view(buffer2, size) == "<mat");
+}
+
 TEST_CASE("MatchingInputs")
 {
   OutputSpec fullySpecified{