Fix undefined behavior in getUniqueEntryID
Changing
static bool getUniqueEntryID
to
inline bool getUniqueEntryID
Exposes the fact that there's some undefined behavior here, and unit
tests start failing, likely due to stack being reused where it
previously wasn't.
This commit cleans up the code to simplify it, and remove the problem.
Tested: Unit tests pass. Good coverage.
Change-Id: I5b9b8e8bb83c656560193e680d246c8513ed6c02
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp
index d2b834a..609ff2c 100644
--- a/redfish-core/lib/log_services.hpp
+++ b/redfish-core/lib/log_services.hpp
@@ -271,21 +271,15 @@
// Entry is formed like "BootID_timestamp" or "BootID_timestamp_index"
inline static bool
getTimestampFromID(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const std::string& entryID, sd_id128_t& bootID,
+ std::string_view entryIDStrView, sd_id128_t& bootID,
uint64_t& timestamp, uint64_t& index)
{
- if (entryID.empty())
- {
- return false;
- }
-
// Convert the unique ID back to a bootID + timestamp to find the entry
- std::string_view entryIDStrView(entryID);
auto underscore1Pos = entryIDStrView.find('_');
if (underscore1Pos == std::string_view::npos)
{
// EntryID has no bootID or timestamp
- messages::resourceNotFound(asyncResp->res, "LogEntry", entryID);
+ messages::resourceNotFound(asyncResp->res, "LogEntry", entryIDStrView);
return false;
}
@@ -294,41 +288,44 @@
// Convert entryIDViewString to BootID
// NOTE: bootID string which needs to be null-terminated for
// sd_id128_from_string()
- std::string bootIDStr(entryID, 0, underscore1Pos);
+ std::string bootIDStr(entryIDStrView.substr(0, underscore1Pos));
if (sd_id128_from_string(bootIDStr.c_str(), &bootID) < 0)
{
- messages::resourceNotFound(asyncResp->res, "LogEntry", entryID);
+ messages::resourceNotFound(asyncResp->res, "LogEntry", entryIDStrView);
return false;
}
// Get the timestamp from entryID
- std::string_view timestampStrView = entryIDStrView;
- timestampStrView.remove_prefix(underscore1Pos + 1);
+ entryIDStrView.remove_prefix(underscore1Pos + 1);
- // Check the index in timestamp
- auto underscore2Pos = timestampStrView.find('_');
- if (underscore2Pos != std::string_view::npos)
+ auto [timestampEnd, tstampEc] = std::from_chars(
+ entryIDStrView.begin(), entryIDStrView.end(), timestamp);
+ if (tstampEc != std::errc())
{
- // Timestamp has an index
- timestampStrView.remove_suffix(timestampStrView.size() -
- underscore2Pos);
- std::string_view indexStr(timestampStrView);
- indexStr.remove_prefix(underscore2Pos + 1);
- auto [ptr, ec] = std::from_chars(indexStr.begin(), indexStr.end(),
- index);
- if (ec != std::errc())
- {
- messages::resourceNotFound(asyncResp->res, "LogEntry", entryID);
- return false;
- }
+ messages::resourceNotFound(asyncResp->res, "LogEntry", entryIDStrView);
+ return false;
}
-
- // Now timestamp has no index
- auto [ptr, ec] = std::from_chars(timestampStrView.begin(),
- timestampStrView.end(), timestamp);
- if (ec != std::errc())
+ entryIDStrView = std::string_view(
+ timestampEnd,
+ static_cast<size_t>(std::distance(timestampEnd, entryIDStrView.end())));
+ if (entryIDStrView.empty())
{
- messages::resourceNotFound(asyncResp->res, "LogEntry", entryID);
+ index = 0U;
+ return true;
+ }
+ // Timestamp might include optional index, if two events happened at the
+ // same "time".
+ if (entryIDStrView[0] != '_')
+ {
+ messages::resourceNotFound(asyncResp->res, "LogEntry", entryIDStrView);
+ return false;
+ }
+ entryIDStrView.remove_prefix(1);
+ auto [ptr, indexEc] = std::from_chars(entryIDStrView.begin(),
+ entryIDStrView.end(), index);
+ if (indexEc != std::errc() || ptr != entryIDStrView.end())
+ {
+ messages::resourceNotFound(asyncResp->res, "LogEntry", entryIDStrView);
return false;
}
return true;