Fix a bunch of warnings
using the list of warnings from here:
https://github.com/lefticus/cppbestpractices/blob/e73393f25a85f83fed7399d8b65cb117d00b2231/02-Use_the_Tools_Available.md#L100
Seems like a good place to start, and would improve things a bit
type-wise. This patchset attempts to correct all the issues in one
shot.
Tested:
It builds. Will test various subsystems that have been touched
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I588c26440e5a97f718a0f0ea74cc84107d53aa1e
diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp
index 9fd35a9..7a63162 100644
--- a/redfish-core/lib/log_services.hpp
+++ b/redfish-core/lib/log_services.hpp
@@ -145,8 +145,8 @@
size_t length = 0;
int ret = 0;
// Get the metadata from the requested field of the journal entry
- ret = sd_journal_get_data(journal, field.data(), (const void **)&data,
- &length);
+ ret = sd_journal_get_data(journal, field.data(),
+ reinterpret_cast<const void **>(&data), &length);
if (ret < 0)
{
return ret;
@@ -159,7 +159,7 @@
static int getJournalMetadata(sd_journal *journal,
const std::string_view &field, const int &base,
- int &contents)
+ long int &contents)
{
int ret = 0;
std::string_view metadata;
@@ -205,13 +205,13 @@
}
static bool getSkipParam(crow::Response &res, const crow::Request &req,
- long &skip)
+ uint64_t &skip)
{
char *skipParam = req.urlParams.get("$skip");
if (skipParam != nullptr)
{
char *ptr = nullptr;
- skip = std::strtol(skipParam, &ptr, 10);
+ skip = std::strtoul(skipParam, &ptr, 10);
if (*skipParam == '\0' || *ptr != '\0')
{
@@ -219,33 +219,26 @@
"$skip");
return false;
}
- if (skip < 0)
- {
-
- messages::queryParameterOutOfRange(res, std::to_string(skip),
- "$skip", "greater than 0");
- return false;
- }
}
return true;
}
-static constexpr const long maxEntriesPerPage = 1000;
+static constexpr const uint64_t maxEntriesPerPage = 1000;
static bool getTopParam(crow::Response &res, const crow::Request &req,
- long &top)
+ uint64_t &top)
{
char *topParam = req.urlParams.get("$top");
if (topParam != nullptr)
{
char *ptr = nullptr;
- top = std::strtol(topParam, &ptr, 10);
+ top = std::strtoul(topParam, &ptr, 10);
if (*topParam == '\0' || *ptr != '\0')
{
messages::queryParameterValueTypeError(res, std::string(topParam),
"$top");
return false;
}
- if (top < 1 || top > maxEntriesPerPage)
+ if (top < 1U || top > maxEntriesPerPage)
{
messages::queryParameterOutOfRange(
@@ -301,7 +294,7 @@
static bool getUniqueEntryID(const std::string &logEntry, std::string &entryID,
const bool firstEntry = true)
{
- static uint64_t prevTs = 0;
+ static time_t prevTs = 0;
static int index = 0;
if (firstEntry)
{
@@ -309,7 +302,7 @@
}
// Get the entry timestamp
- uint64_t curTs = 0;
+ std::time_t curTs = 0;
std::tm timeStruct = {};
std::istringstream entryStream(logEntry);
if (entryStream >> std::get_time(&timeStruct, "%Y-%m-%dT%H:%M:%S"))
@@ -338,7 +331,7 @@
}
static bool getTimestampFromID(crow::Response &res, const std::string &entryID,
- uint64_t ×tamp, uint16_t &index)
+ uint64_t ×tamp, uint64_t &index)
{
if (entryID.empty())
{
@@ -359,12 +352,12 @@
{
index = std::stoul(std::string(indexStr), &pos);
}
- catch (std::invalid_argument)
+ catch (std::invalid_argument &)
{
messages::resourceMissingAtURI(res, entryID);
return false;
}
- catch (std::out_of_range)
+ catch (std::out_of_range &)
{
messages::resourceMissingAtURI(res, entryID);
return false;
@@ -381,12 +374,12 @@
{
timestamp = std::stoull(std::string(tsStr), &pos);
}
- catch (std::invalid_argument)
+ catch (std::invalid_argument &)
{
messages::resourceMissingAtURI(res, entryID);
return false;
}
- catch (std::out_of_range)
+ catch (std::out_of_range &)
{
messages::resourceMissingAtURI(res, entryID);
return false;
@@ -685,8 +678,8 @@
const std::vector<std::string> ¶ms) override
{
std::shared_ptr<AsyncResp> asyncResp = std::make_shared<AsyncResp>(res);
- long skip = 0;
- long top = maxEntriesPerPage; // Show max entries by default
+ uint64_t skip = 0;
+ uint64_t top = maxEntriesPerPage; // Show max entries by default
if (!getSkipParam(asyncResp->res, req, skip))
{
return;
@@ -912,7 +905,6 @@
uint32_t *id;
std::time_t timestamp;
std::string *severity, *message;
- bool *resolved;
for (auto &propertyMap : interfaceMap.second)
{
if (propertyMap.first == "Id")
@@ -933,15 +925,16 @@
{
messages::propertyMissing(asyncResp->res,
"Timestamp");
+ continue;
}
// Retrieve Created property with format:
// yyyy-mm-ddThh:mm:ss
std::chrono::milliseconds chronoTimeStamp(
*millisTimeStamp);
- timestamp =
- std::chrono::duration_cast<
- std::chrono::seconds>(chronoTimeStamp)
- .count();
+ timestamp = std::chrono::duration_cast<
+ std::chrono::duration<int>>(
+ chronoTimeStamp)
+ .count();
}
else if (propertyMap.first == "Severity")
{
@@ -1038,7 +1031,6 @@
uint32_t *id;
std::time_t timestamp;
std::string *severity, *message;
- bool *resolved;
for (auto &propertyMap : resp)
{
if (propertyMap.first == "Id")
@@ -1057,14 +1049,15 @@
{
messages::propertyMissing(asyncResp->res,
"Timestamp");
+ continue;
}
// Retrieve Created property with format:
// yyyy-mm-ddThh:mm:ss
std::chrono::milliseconds chronoTimeStamp(
*millisTimeStamp);
timestamp =
- std::chrono::duration_cast<std::chrono::seconds>(
- chronoTimeStamp)
+ std::chrono::duration_cast<
+ std::chrono::duration<int>>(chronoTimeStamp)
.count();
}
else if (propertyMap.first == "Severity")
@@ -1087,6 +1080,10 @@
}
}
}
+ if (id == nullptr || message == nullptr || severity == nullptr)
+ {
+ return;
+ }
asyncResp->res.jsonValue = {
{"@odata.type", "#LogEntry.v1_4_0.LogEntry"},
{"@odata.context", "/redfish/v1/"
@@ -1249,7 +1246,7 @@
}
// Get the severity from the PRIORITY field
- int severity = 8; // Default to an invalid priority
+ long int severity = 8; // Default to an invalid priority
ret = getJournalMetadata(journal, "PRIORITY", 10, severity);
if (ret < 0)
{
@@ -1302,8 +1299,8 @@
{
std::shared_ptr<AsyncResp> asyncResp = std::make_shared<AsyncResp>(res);
static constexpr const long maxEntriesPerPage = 1000;
- long skip = 0;
- long top = maxEntriesPerPage; // Show max entries by default
+ uint64_t skip = 0;
+ uint64_t top = maxEntriesPerPage; // Show max entries by default
if (!getSkipParam(asyncResp->res, req, skip))
{
return;
@@ -1415,7 +1412,7 @@
const std::string &entryID = params[0];
// Convert the unique ID back to a timestamp to find the entry
uint64_t ts = 0;
- uint16_t index = 0;
+ uint64_t index = 0;
if (!getTimestampFromID(asyncResp->res, entryID, ts, index))
{
return;
@@ -1437,7 +1434,7 @@
std::string idStr;
bool firstEntry = true;
ret = sd_journal_seek_realtime_usec(journal.get(), ts);
- for (int i = 0; i <= index; i++)
+ for (uint64_t i = 0; i <= index; i++)
{
sd_journal_next(journal.get());
if (!getUniqueEntryID(journal.get(), idStr, firstEntry))
@@ -1652,7 +1649,7 @@
messages::internalError(asyncResp->res);
return;
}
- const uint8_t logId = std::atoi(params[0].c_str());
+ const int logId = std::atoi(params[0].c_str());
auto getStoredLogCallback = [asyncResp, logId](
const boost::system::error_code ec,
const std::variant<std::string> &resp) {
@@ -1728,9 +1725,9 @@
return;
}
// Make this static so it survives outside this method
- static boost::asio::deadline_timer timeout(*req.ioService);
+ static boost::asio::steady_timer timeout(*req.ioService);
- timeout.expires_from_now(boost::posix_time::seconds(30));
+ timeout.expires_after(std::chrono::seconds(30));
timeout.async_wait([asyncResp](const boost::system::error_code &ec) {
onDemandLogMatcher = nullptr;
if (ec)
@@ -1751,12 +1748,8 @@
auto onDemandLogMatcherCallback = [asyncResp](
sdbusplus::message::message &m) {
BMCWEB_LOG_DEBUG << "OnDemand log available match fired";
- boost::system::error_code ec;
- timeout.cancel(ec);
- if (ec)
- {
- BMCWEB_LOG_ERROR << "error canceling timer " << ec;
- }
+ timeout.cancel();
+
sdbusplus::message::object_path objPath;
boost::container::flat_map<
std::string, boost::container::flat_map<
@@ -1825,13 +1818,8 @@
{
messages::internalError(asyncResp->res);
}
- boost::system::error_code timeoutec;
- timeout.cancel(timeoutec);
- if (timeoutec)
- {
- BMCWEB_LOG_ERROR << "error canceling timer "
- << timeoutec;
- }
+
+ timeout.cancel();
onDemandLogMatcher = nullptr;
return;
}