Simplify fillMessageArgs
The aforementioned function does a lot of reconstruction of strings as
args are filled in. This results in the end of the string being copied
many times (N). Replace the algorithm with one that builds a new
string, using reserve (which is good practice) and is also capable of
returning errors in the case of bad entries. fillMessageArgs now
returns a string instead of trying to do things in place, which avoids
the initial std::string construction, so we should be net the same here.
Given this new algorithm can now detect failures in parsing (ie, trying
to parse %1 with no arguments) add unit tests for coverage of that, and
modify event manager slightly to handle errors.
Tested: Unit tests pass. Pretty good coverage of this stuff.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I16f28a2ac78580fb35266561f5ae38078b471989
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index 6b3f84f..d581aeb 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -235,15 +235,17 @@
// Get the Message from the MessageRegistry
const registries::Message* message = registries::formatMessage(messageID);
- std::string msg;
- std::string severity;
- if (message != nullptr)
+ if (message == nullptr)
{
- msg = message->message;
- severity = message->messageSeverity;
+ return -1;
}
- redfish::registries::fillMessageArgs(messageArgs, msg);
+ std::string msg =
+ redfish::registries::fillMessageArgs(messageArgs, message->message);
+ if (msg.empty())
+ {
+ return -1;
+ }
// Get the Created time from the timestamp. The log timestamp is in
// RFC3339 format which matches the Redfish format except for the
@@ -258,7 +260,7 @@
// Fill in the log entry with the gathered data
logEntryJson["EventId"] = logEntryID;
logEntryJson["EventType"] = "Event";
- logEntryJson["Severity"] = std::move(severity);
+ logEntryJson["Severity"] = message->messageSeverity;
logEntryJson["Message"] = std::move(msg);
logEntryJson["MessageId"] = messageID;
logEntryJson["MessageArgs"] = messageArgs;
diff --git a/redfish-core/include/registries.hpp b/redfish-core/include/registries.hpp
index a995d76..4247aa8 100644
--- a/redfish-core/include/registries.hpp
+++ b/redfish-core/include/registries.hpp
@@ -16,7 +16,10 @@
#pragma once
#include <array>
+#include <charconv>
#include <cstddef>
+#include <iostream>
+#include <numeric>
#include <span>
#include <string>
#include <string_view>
@@ -50,20 +53,40 @@
};
using MessageEntry = std::pair<const char*, const Message>;
-inline void fillMessageArgs(const std::span<const std::string_view> messageArgs,
- std::string& msg)
+inline std::string
+ fillMessageArgs(const std::span<const std::string_view> messageArgs,
+ std::string_view msg)
{
- int i = 0;
- for (const std::string_view& messageArg : messageArgs)
+ std::string ret;
+ size_t reserve = msg.size();
+ for (const std::string_view& arg : messageArgs)
{
- std::string argStr = "%" + std::to_string(i + 1);
- size_t argPos = msg.find(argStr);
- if (argPos != std::string::npos)
- {
- msg.replace(argPos, argStr.length(), messageArg);
- }
- i++;
+ reserve += arg.size();
}
+ ret.reserve(reserve);
+
+ for (size_t stringIndex = msg.find('%'); stringIndex != std::string::npos;
+ stringIndex = msg.find('%'))
+ {
+ ret += msg.substr(0, stringIndex);
+ msg.remove_prefix(stringIndex + 1);
+ size_t number = 0;
+ auto it = std::from_chars(msg.data(), &*msg.end(), number);
+ if (it.ec != std::errc())
+ {
+ return "";
+ }
+ msg.remove_prefix(1);
+ // Redfish message args are 1 indexed.
+ number--;
+ if (number >= messageArgs.size())
+ {
+ return "";
+ }
+ ret += messageArgs[number];
+ }
+ ret += msg;
+ return ret;
}
} // namespace redfish::registries
diff --git a/redfish-core/ut/registries_test.cpp b/redfish-core/ut/registries_test.cpp
index 87b92dc..95a093e 100644
--- a/redfish-core/ut/registries_test.cpp
+++ b/redfish-core/ut/registries_test.cpp
@@ -13,17 +13,13 @@
TEST(FillMessageArgs, ArgsAreFilledCorrectly)
{
- std::string toFill("%1");
- fillMessageArgs({{"foo"}}, toFill);
- EXPECT_EQ(toFill, "foo");
-
- toFill = "";
- fillMessageArgs({}, toFill);
- EXPECT_EQ(toFill, "");
-
- toFill = "%1, %2";
- fillMessageArgs({{"foo", "bar"}}, toFill);
- EXPECT_EQ(toFill, "foo, bar");
+ EXPECT_EQ(fillMessageArgs({{"foo"}}, "%1"), "foo");
+ EXPECT_EQ(fillMessageArgs({}, ""), "");
+ EXPECT_EQ(fillMessageArgs({{"foo", "bar"}}, "%1, %2"), "foo, bar");
+ EXPECT_EQ(fillMessageArgs({{"foo"}}, "%1 bar"), "foo bar");
+ EXPECT_EQ(fillMessageArgs({}, "%1"), "");
+ EXPECT_EQ(fillMessageArgs({}, "%"), "");
+ EXPECT_EQ(fillMessageArgs({}, "%foo"), "");
}
} // namespace
-} // namespace redfish::registries
\ No newline at end of file
+} // namespace redfish::registries