Improve loops & fix cpp check warning
- This commit improves certain while loops to range based for loops.
- This commit also fixes the cppcheck warning that mentions about
performance issues when using postfix operators on non-primitive
types.
Tested By:
- A function is unittested.
- GET on both EthernetInterfaces & certificate service
looks good without any issues.
Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
Change-Id: I85420f7bf9af45a97e1a93b916f292c2516f5802
diff --git a/include/dbus_utility.hpp b/include/dbus_utility.hpp
index e1360f7..8ba9a57 100644
--- a/include/dbus_utility.hpp
+++ b/include/dbus_utility.hpp
@@ -17,6 +17,7 @@
#include <sdbusplus/message.hpp>
+#include <filesystem>
#include <regex>
namespace dbus
@@ -54,27 +55,21 @@
inline bool getNthStringFromPath(const std::string& path, int index,
std::string& result)
{
- int count = 0;
- std::string::const_iterator first = path.begin();
- std::string::const_iterator last = path.end();
- for (std::string::const_iterator it = path.begin(); it < path.end(); it++)
+ if (index < 0)
{
- // skip first character as it's either a leading slash or the first
- // character in the word
- if (it == path.begin())
+ return false;
+ }
+
+ std::filesystem::path p1(path);
+ int count = -1;
+ for (auto const& element : p1)
+ {
+ if (element.has_filename())
{
- continue;
- }
- if (*it == '/')
- {
- count++;
+ ++count;
if (count == index)
{
- first = it;
- }
- if (count == index + 1)
- {
- last = it;
+ result = element.stem().string();
break;
}
}
@@ -83,12 +78,7 @@
{
return false;
}
- if (first != path.begin())
- {
- first++;
- }
- result = path.substr(static_cast<size_t>(first - path.begin()),
- static_cast<size_t>(last - first));
+
return true;
}
diff --git a/include/ut/dbus_utility_test.cpp b/include/ut/dbus_utility_test.cpp
new file mode 100644
index 0000000..d4342a5
--- /dev/null
+++ b/include/ut/dbus_utility_test.cpp
@@ -0,0 +1,36 @@
+#include <boost/algorithm/string.hpp>
+#include <boost/container/flat_set.hpp>
+#include <dbus_singleton.hpp>
+#include <dbus_utility.hpp>
+
+#include "gmock/gmock.h"
+
+TEST(DbusUtility, getNthStringFromPathGoodTest)
+{
+ std::string path("/0th/1st/2nd/3rd");
+ std::string result;
+ EXPECT_TRUE(dbus::utility::getNthStringFromPath(path, 0, result));
+ EXPECT_EQ(result, "0th");
+ EXPECT_TRUE(dbus::utility::getNthStringFromPath(path, 1, result));
+ EXPECT_EQ(result, "1st");
+ EXPECT_TRUE(dbus::utility::getNthStringFromPath(path, 2, result));
+ EXPECT_EQ(result, "2nd");
+ EXPECT_TRUE(dbus::utility::getNthStringFromPath(path, 3, result));
+ EXPECT_EQ(result, "3rd");
+ EXPECT_FALSE(dbus::utility::getNthStringFromPath(path, 4, result));
+}
+
+TEST(DbusUtility, getNthStringFromPathBadTest)
+{
+ std::string path("////0th///1st//\2nd///3rd?/");
+ std::string result;
+ EXPECT_TRUE(dbus::utility::getNthStringFromPath(path, 0, result));
+ EXPECT_EQ(result, "0th");
+ EXPECT_TRUE(dbus::utility::getNthStringFromPath(path, 1, result));
+ EXPECT_EQ(result, "1st");
+ EXPECT_TRUE(dbus::utility::getNthStringFromPath(path, 2, result));
+ EXPECT_EQ(result, "\2nd");
+ EXPECT_TRUE(dbus::utility::getNthStringFromPath(path, 3, result));
+ EXPECT_EQ(result, "3rd?");
+ EXPECT_FALSE(dbus::utility::getNthStringFromPath(path, -1, result));
+}
diff --git a/meson.build b/meson.build
index 5079bfc..6b449cc 100644
--- a/meson.build
+++ b/meson.build
@@ -319,7 +319,8 @@
srcfiles_bmcweb = ['src/webserver_main.cpp','redfish-core/src/error_messages.cpp',
'redfish-core/src/utils/json_utils.cpp']
-srcfiles_unittest = ['redfish-core/ut/privileges_test.cpp',
+srcfiles_unittest = ['include/ut/dbus_utility_test.cpp',
+ 'redfish-core/ut/privileges_test.cpp',
'redfish-core/ut/lock_test.cpp',
'http/ut/utility_test.cpp']
diff --git a/redfish-core/lib/certificate_service.hpp b/redfish-core/lib/certificate_service.hpp
index 5dc1e1f..81d9d05 100644
--- a/redfish-core/lib/certificate_service.hpp
+++ b/redfish-core/lib/certificate_service.hpp
@@ -516,7 +516,7 @@
std::string_view::iterator tokenBegin = i;
while (i != value.end() && *i != '=')
{
- i++;
+ ++i;
}
if (i == value.end())
{
@@ -524,11 +524,11 @@
}
const std::string_view key(tokenBegin,
static_cast<size_t>(i - tokenBegin));
- i++;
+ ++i;
tokenBegin = i;
while (i != value.end() && *i != ',')
{
- i++;
+ ++i;
}
const std::string_view val(tokenBegin,
static_cast<size_t>(i - tokenBegin));
@@ -559,7 +559,7 @@
// skip comma character
if (i != value.end())
{
- i++;
+ ++i;
}
}
}
diff --git a/redfish-core/lib/ethernet.hpp b/redfish-core/lib/ethernet.hpp
index 55f1cf5..48b69a2 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -1375,34 +1375,26 @@
boost::container::flat_set<IPv4AddressData>::const_iterator
getNextStaticIpEntry(
- boost::container::flat_set<IPv4AddressData>::const_iterator head,
+ const boost::container::flat_set<IPv4AddressData>::const_iterator&
+ head,
const boost::container::flat_set<IPv4AddressData>::const_iterator&
end)
{
- for (; head != end; head++)
- {
- if (head->origin == "Static")
- {
- return head;
- }
- }
- return end;
+ return std::find_if(head, end, [](const IPv4AddressData& value) {
+ return value.origin == "Static";
+ });
}
boost::container::flat_set<IPv6AddressData>::const_iterator
getNextStaticIpEntry(
- boost::container::flat_set<IPv6AddressData>::const_iterator head,
+ const boost::container::flat_set<IPv6AddressData>::const_iterator&
+ head,
const boost::container::flat_set<IPv6AddressData>::const_iterator&
end)
{
- for (; head != end; head++)
- {
- if (head->origin == "Static")
- {
- return head;
- }
- }
- return end;
+ return std::find_if(head, end, [](const IPv6AddressData& value) {
+ return value.origin == "Static";
+ });
}
void handleIPv4StaticPatch(
diff --git a/redfish-core/lib/managers.hpp b/redfish-core/lib/managers.hpp
index 26c2125..c621de3 100644
--- a/redfish-core/lib/managers.hpp
+++ b/redfish-core/lib/managers.hpp
@@ -1520,7 +1520,7 @@
std::string& type = containerPair.first;
for (nlohmann::json::iterator it = container->begin();
- it != container->end(); it++)
+ it != container->end(); ++it)
{
const auto& name = it.key();
BMCWEB_LOG_DEBUG << "looking for " << name;