Skip to content

Commit c4f4364

Browse files
ktfktf
authored
DPL: improve DataSpecUtils::describe API in case of buffers (AliceO2Group#14238)
Just like snprintf, it makes sense to return the size of the formatted output.
1 parent 1c7a558 commit c4f4364

File tree

3 files changed

+88
-42
lines changed

3 files changed

+88
-42
lines changed

‎Framework/Core/include/Framework/DataSpecUtils.h‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,12 @@
1818

1919
#include <optional>
2020

21-
namespace o2
22-
{
23-
namespace framework
21+
namespace o2::framework
2422
{
2523

24+
template <typename T>
25+
concept HasMatcher = requires(T& t) { t.matcher; };
26+
2627
struct DataSpecUtils {
2728
/// @return true if a given InputSpec @a spec matches with a @a target ConcreteDataMatcher
2829
static bool match(InputSpec const& spec, ConcreteDataMatcher const& target);
@@ -152,10 +153,8 @@ struct DataSpecUtils {
152153
static bool validate(OutputSpec const& output);
153154

154155
/// Same as the other describe, but uses a buffer to reduce memory churn.
155-
static void describe(char* buffer, size_t size, InputSpec const& spec);
156-
157-
/// Same as the other describe, but uses a buffer to reduce memory churn.
158-
static void describe(char* buffer, size_t size, OutputSpec const& spec);
156+
template <HasMatcher T>
157+
static size_t describe(char* buffer, size_t size, T const& spec);
159158

160159
/// If possible extract the ConcreteDataMatcher from an InputSpec. This
161160
/// can be done either if the InputSpec is defined in terms for a ConcreteDataMatcher
@@ -250,6 +249,6 @@ struct DataSpecUtils {
250249
static void updateOutputList(std::vector<OutputSpec>& list, OutputSpec&& input);
251250
};
252251

253-
} // namespace framework
254-
} // namespace o2
252+
} // namespace o2::framework
253+
255254
#endif // FRAMEWORK_DATASPECUTILS_H

‎Framework/Core/src/DataSpecUtils.cxx‎

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@
1515
#include "Framework/RuntimeError.h"
1616
#include "Headers/DataHeaderHelpers.h"
1717

18+
#include <fmt/base.h>
1819
#include <fmt/format.h>
1920
#include <sstream>
2021
#include <cstring>
2122
#include <cinttypes>
2223
#include <regex>
24+
#include <stdexcept>
2325

2426
namespace o2::framework
2527
{
@@ -87,39 +89,29 @@ std::string DataSpecUtils::describe(OutputSpec const& spec)
8789
spec.matcher);
8890
}
8991

90-
void DataSpecUtils::describe(char* buffer, size_t size, InputSpec const& spec)
91-
{
92-
if (auto concrete = std::get_if<ConcreteDataMatcher>(&spec.matcher)) {
93-
char origin[5];
94-
origin[4] = 0;
95-
char description[17];
96-
description[16] = 0;
97-
snprintf(buffer, size, "%s/%s/%" PRIu32, (strncpy(origin, concrete->origin.str, 4), origin),
98-
(strncpy(description, concrete->description.str, 16), description), concrete->subSpec);
99-
} else if (auto matcher = std::get_if<DataDescriptorMatcher>(&spec.matcher)) {
100-
std::ostringstream ss;
101-
ss << "<matcher query: " << *matcher << ">";
102-
strncpy(buffer, ss.str().c_str(), size - 1);
103-
} else {
104-
throw runtime_error("Unsupported InputSpec");
105-
}
106-
}
107-
108-
void DataSpecUtils::describe(char* buffer, size_t size, OutputSpec const& spec)
109-
{
110-
if (auto concrete = std::get_if<ConcreteDataMatcher>(&spec.matcher)) {
111-
char origin[5];
112-
origin[4] = 0;
113-
char description[17];
114-
description[16] = 0;
115-
snprintf(buffer, size, "%s/%s/%" PRIu32, (strncpy(origin, concrete->origin.str, 4), origin),
116-
(strncpy(description, concrete->description.str, 16), description), concrete->subSpec);
117-
} else if (auto concrete = std::get_if<ConcreteDataTypeMatcher>(&spec.matcher)) {
118-
fmt::format_to(buffer, "<matcher query: {}/{}>", concrete->origin, concrete->description);
119-
} else {
120-
throw runtime_error("Unsupported OutputSpec");
121-
}
122-
}
92+
template <HasMatcher T>
93+
size_t DataSpecUtils::describe(char* buffer, size_t size, T const& spec)
94+
{
95+
auto result = std::visit(overloaded{
96+
[buffer, size](ConcreteDataMatcher const& concrete) -> fmt::format_to_n_result<char*> {
97+
return fmt::format_to_n(buffer, size - 1, "{:.4}/{:.16}/{}", concrete.origin.str, concrete.description.str, concrete.subSpec);
98+
},
99+
[buffer, size](ConcreteDataTypeMatcher const& concrete) -> fmt::format_to_n_result<char*> {
100+
return fmt::format_to_n(buffer, size - 1, "<matcher query: {}/{}>", concrete.origin, concrete.description);
101+
},
102+
[buffer, size](DataDescriptorMatcher const& matcher) -> fmt::format_to_n_result<char*> {
103+
std::ostringstream ss;
104+
ss << "<matcher query: " << matcher << ">";
105+
return fmt::format_to_n(buffer, size - 1, "{}", ss.str());
106+
},
107+
[](...) -> fmt::format_to_n_result<char*> { throw std::runtime_error("Unsupported Input / Output Spec"); }},
108+
spec.matcher);
109+
*result.out = '\0';
110+
return result.out - buffer;
111+
}
112+
113+
template size_t DataSpecUtils::describe(char* buffer, size_t size, InputSpec const& spec);
114+
template size_t DataSpecUtils::describe(char* buffer, size_t size, OutputSpec const& spec);
123115

124116
std::string DataSpecUtils::label(InputSpec const& spec)
125117
{

‎Framework/Core/test/unittest_DataSpecUtils.cxx‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ TEST_CASE("ConcreteData")
4242
CHECK(std::string(concrete.description.as<std::string>()) == "FOOO");
4343
CHECK(concrete.subSpec == 1);
4444
CHECK(DataSpecUtils::describe(spec) == "TEST/FOOO/1");
45+
CHECK(DataSpecUtils::describe(spec) == "TEST/FOOO/1");
4546
CHECK(*DataSpecUtils::getOptionalSubSpec(spec) == 1);
4647

4748
ConcreteDataTypeMatcher dataType = DataSpecUtils::asConcreteDataTypeMatcher(spec);
@@ -59,6 +60,44 @@ TEST_CASE("ConcreteData")
5960
}
6061
}
6162

63+
TEST_CASE("DescribeUsingBuffer")
64+
{
65+
o2::framework::clean_all_runtime_errors();
66+
OutputSpec spec{
67+
"TEST",
68+
"FOOO",
69+
1,
70+
Lifetime::Timeframe};
71+
72+
InputSpec inputSpec{
73+
"binding",
74+
"TEST",
75+
"FOOO",
76+
1,
77+
Lifetime::Timeframe};
78+
79+
REQUIRE(DataSpecUtils::validate(inputSpec));
80+
81+
{
82+
char buffer[1024];
83+
84+
ConcreteDataMatcher concrete = DataSpecUtils::asConcreteDataMatcher(spec);
85+
CHECK(std::string(concrete.origin.as<std::string>()) == "TEST");
86+
CHECK(std::string(concrete.description.as<std::string>()) == "FOOO");
87+
CHECK(concrete.subSpec == 1);
88+
auto size = DataSpecUtils::describe(buffer, 1024, spec);
89+
CHECK(std::string_view(buffer, size) == "TEST/FOOO/1");
90+
size = DataSpecUtils::describe(buffer, 1024, spec);
91+
CHECK(std::string_view(buffer, size) == "TEST/FOOO/1");
92+
CHECK(*DataSpecUtils::getOptionalSubSpec(spec) == 1);
93+
94+
char buffer2[1024];
95+
size = DataSpecUtils::describe(buffer2, 5, spec);
96+
// We always nullterminate the string
97+
CHECK(std::string_view(buffer2, size) == "TEST");
98+
}
99+
}
100+
62101
TEST_CASE("WithWildCards")
63102
{
64103
OutputSpec spec{
@@ -78,6 +117,22 @@ TEST_CASE("WithWildCards")
78117
CHECK(DataSpecUtils::getOptionalSubSpec(spec) == std::nullopt);
79118
}
80119

120+
TEST_CASE("WithWildCardsBuffer")
121+
{
122+
char buffer[1024];
123+
OutputSpec spec{
124+
{"TEST", "FOOO"},
125+
Lifetime::Timeframe};
126+
127+
auto size = DataSpecUtils::describe(buffer, 1024, spec);
128+
CHECK(std::string_view(buffer, size) == "<matcher query: TEST/FOOO>");
129+
130+
char buffer2[1024];
131+
size = DataSpecUtils::describe(buffer2, 5, spec);
132+
// We always null terminate the buffer.
133+
CHECK(std::string_view(buffer2, size) == "<mat");
134+
}
135+
81136
TEST_CASE("MatchingInputs")
82137
{
83138
OutputSpec fullySpecified{

0 commit comments

Comments
 (0)