Reduce journal logs

Presently the code was creating the elog during reading of
the values from the config parser which creates
the journal log internally("Internal error Occured")

Apart from creating elog,Code also logs into the journal
more descriptive message saying which section and key
was not found.

The code which catches the exception that also logs
into the journal.

In the changed approach instead of throwing exception
we are returning the return code as not finding a section
or key in the network config file is not an error.

Change-Id: Ib89a3af6541fdf93fb5fa9a8e583d6c6ed88da79
Signed-off-by: Ratan Gupta <ratagupt@in.ibm.com>

8 files changed, 113 insertions, 95 deletions

diff --git a/config_parser.cpp b/config_parser.cpp
index 9ab601c..3852b6c 100644
--- a/config_parser.cpp
+++ b/config_parser.cpp
@@ -1,7 +1,5 @@
 #include "config_parser.hpp"
-#include "xyz/openbmc_project/Common/error.hpp"
 #include <phosphor-logging/log.hpp>
-#include <phosphor-logging/elog-errors.hpp>
 
 #include <fstream>
 #include <string>
@@ -18,7 +16,6 @@ namespace config
 {
 
 using namespace phosphor::logging;
-using namespace sdbusplus::xyz::openbmc_project::Common::Error;
 
 Parser::Parser(const fs::path& filePath)
 {
@@ -26,58 +23,66 @@ Parser::Parser(const fs::path& filePath)
 }
 
 
-KeyValues Parser::getSection(const std::string& section)
+std::tuple<ReturnCode, KeyValueMap> Parser::getSection(const std::string& section)
 {
     auto it = sections.find(section);
     if (it == sections.end())
     {
-        log<level::ERR>("ConfigParser: Section not found",
-                        entry("SECTION=%s",section));
-        elog<InternalFailure>();
+        KeyValueMap keyValues;
+        return std::make_tuple(ReturnCode::SECTION_NOT_FOUND,
+                               std::move(keyValues));
     }
-    return it->second;
+
+    return std::make_tuple(ReturnCode::SUCCESS, it->second);
 }
 
-std::vector<std::string> Parser::getValues(const std::string& section,
-                                           const std::string& key)
+std::tuple<ReturnCode, ValueList> Parser::getValues(const std::string& section,
+                                                    const std::string& key)
 {
-    std::vector<std::string> values;
-    auto keyValues = getSection(section);
+    ValueList values;
+    KeyValueMap keyValues {};
+    auto rc = ReturnCode::SUCCESS;
+
+    std::tie(rc, keyValues) = getSection(section);
+    if (rc != ReturnCode::SUCCESS)
+    {
+        return std::make_tuple(rc, std::move(values));
+    }
+
     auto it = keyValues.find(key);
     if (it == keyValues.end())
     {
-        log<level::ERR>("ConfigParser: Key not found",
-                        entry("KEY=%s",key));
-        elog<InternalFailure>();
+        return std::make_tuple(ReturnCode::KEY_NOT_FOUND, std::move(values));
     }
+
     for (; it != keyValues.end() && key == it->first; it++)
     {
         values.push_back(it->second);
     }
-    return values;
+
+    return std::make_tuple(ReturnCode::SUCCESS, std::move(values));
 }
 
 
 bool Parser::isValueExist(const std::string& section, const std::string& key,
                           const std::string& value)
 {
-    try
-    {
-        auto values = getValues(section, key);
-        auto it = std::find(values.begin(), values.end(), value);
-        return it != std::end(values) ? true : false;
-    }
-    catch (InternalFailure& e)
+    auto rc = ReturnCode::SUCCESS;
+    ValueList values;
+    std::tie(rc, values) = getValues(section, key);
+
+    if (rc != ReturnCode::SUCCESS)
     {
-        commit<InternalFailure>();
+        return false;
     }
-    return false;
+    auto it = std::find(values.begin(), values.end(), value);
+    return it != std::end(values) ? true : false;
 }
 
 void Parser::setValue(const std::string& section, const std::string& key,
                       const std::string& value)
 {
-    KeyValues values;
+    KeyValueMap values;
     auto it = sections.find(section);
     if (it != sections.end())
     {
diff --git a/config_parser.hpp b/config_parser.hpp
index bf5cb2d..f3e4a70 100644
--- a/config_parser.hpp
+++ b/config_parser.hpp
@@ -2,6 +2,7 @@
 
 #include <string>
 #include <map>
+#include <tuple>
 #include <unordered_map>
 #include <vector>
 #include <experimental/filesystem>
@@ -14,9 +15,18 @@ namespace config
 {
 
 using Section = std::string;
-using KeyValues =  std::multimap<std::string, std::string>;
+using KeyValueMap =  std::multimap<std::string, std::string>;
+using ValueList = std::vector<std::string>;
+
 namespace fs = std::experimental::filesystem;
 
+enum class ReturnCode
+{
+    SUCCESS = 0x0,
+    SECTION_NOT_FOUND = 0x1,
+    KEY_NOT_FOUND = 0x2,
+};
+
 class Parser
 {
     public:
@@ -32,11 +42,13 @@ class Parser
         /** @brief Get the values of the given key and section.
          *  @param[in] section - section name.
          *  @param[in] key - key to look for.
-         *  @returns the values associated with the key.
+         *  @returns the tuple of return code and the
+         *           values associated with the key.
          */
 
-        std::vector<std::string> getValues(const std::string& section,
-                                           const std::string& key);
+        std::tuple<ReturnCode, ValueList> getValues(
+                const std::string& section,
+                const std::string& key);
 
         /** @brief Set the value of the given key and section.
          *  @param[in] section - section name.
@@ -64,10 +76,10 @@ class Parser
 
         /** @brief Get all the key values of the given section.
          *  @param[in] section - section name.
-         *  @returns the map of the key and value.
+         *  @returns the tuple of return code and the map of (key,value).
          */
 
-        KeyValues getSection(const std::string& section);
+        std::tuple<ReturnCode, KeyValueMap> getSection(const std::string& section);
 
         /** @brief checks that whether the value exist in the
          *         given section.
@@ -80,7 +92,7 @@ class Parser
         bool isValueExist(const std::string& section, const std::string& key,
                           const std::string& value);
 
-        std::unordered_map<Section, KeyValues> sections;
+        std::unordered_map<Section, KeyValueMap> sections;
         fs::path filePath;
 };
 
diff --git a/ethernet_interface.cpp b/ethernet_interface.cpp
index 15f40fb..7d13c1c 100644
--- a/ethernet_interface.cpp
+++ b/ethernet_interface.cpp
@@ -398,14 +398,14 @@ ServerList EthernetInterface::getNameServerFromConf()
                            systemd::config::networkFileSuffix;
     confPath /= fileName;
     ServerList servers;
-    try
-    {
-        config::Parser parser(confPath.string());
-        servers = parser.getValues("Network", "DNS");
-    }
-    catch (InternalFailure& e)
+    config::Parser parser(confPath.string());
+    auto rc = config::ReturnCode::SUCCESS;
+
+    std::tie(rc, servers) = parser.getValues("Network", "DNS");
+    if (rc != config::ReturnCode::SUCCESS)
     {
-        log<level::INFO>("Exception getting DNS value from conf file");
+        log<level::DEBUG>("Unable to get the value for network[DNS]",
+                          entry("RC=%d", rc));
     }
     return servers;
 }
@@ -484,18 +484,20 @@ ServerList EthernetInterface::getNTPServersFromConf()
     fs::path confPath = manager.getConfDir();
 
     std::string fileName = systemd::config::networkFilePrefix + interfaceName() +
-                           systemd::config::networkFileSuffix;
+        systemd::config::networkFileSuffix;
     confPath /= fileName;
+
     ServerList servers;
-    try
-    {
-        config::Parser parser(confPath.string());
-        servers = parser.getValues("Network", "NTP");
-    }
-    catch (InternalFailure& e)
+    config::Parser parser(confPath.string());
+    auto rc = config::ReturnCode::SUCCESS;
+
+    std::tie(rc, servers) = parser.getValues("Network", "NTP");
+    if (rc != config::ReturnCode::SUCCESS)
     {
-        log<level::INFO>("Unable to find the NTP server configuration.");
+        log<level::DEBUG>("Unable to get the value for Network[NTP]",
+                          entry("rc=%d", rc));
     }
+
     return servers;
 }
 
diff --git a/test/test_config_parser.cpp b/test/test_config_parser.cpp
index a07390b..1c902d2 100644
--- a/test/test_config_parser.cpp
+++ b/test/test_config_parser.cpp
@@ -46,17 +46,20 @@ class TestConfigParser : public testing::Test
 
 TEST_F(TestConfigParser, ReadConfigDataFromFile)
 {
-    auto values = parser.getValues("Network", "DHCP");
+    config::ReturnCode rc = config::ReturnCode::SUCCESS;
+    config::ValueList values;
+
+    std::tie(rc, values) = parser.getValues("Network", "DHCP");
     std::string expectedValue = "true";
     bool found = isValueFound(values, expectedValue);
     EXPECT_EQ(found, true);
 
-    values = parser.getValues("DHCP", "ClientIdentifier");
+    std::tie(rc, values) = parser.getValues("DHCP", "ClientIdentifier");
     expectedValue = "mac";
     found = isValueFound(values, expectedValue);
     EXPECT_EQ(found, true);
 
-    values = parser.getValues("Match", "Name");
+    std::tie(rc, values) = parser.getValues("Match", "Name");
     expectedValue = "eth0";
     found = isValueFound(values, expectedValue);
     EXPECT_EQ(found, true);
@@ -64,35 +67,20 @@ TEST_F(TestConfigParser, ReadConfigDataFromFile)
 
 TEST_F(TestConfigParser, SectionNotExist)
 {
-    using namespace sdbusplus::xyz::openbmc_project::Common::Error;
-    bool caughtException = false;
-    try
-    {
-        parser.getValues("abc", "ipaddress");
-    }
-    catch (const std::exception& e)
-    {
-        caughtException = true;
-    }
-    EXPECT_EQ(true, caughtException);
+    config::ReturnCode rc = config::ReturnCode::SUCCESS;
+    config::ValueList values;
+    std::tie(rc, values) = parser.getValues("abc", "ipaddress");
+    EXPECT_EQ(config::ReturnCode::SECTION_NOT_FOUND, rc);
 }
 
 TEST_F(TestConfigParser, KeyNotFound)
 {
-    using namespace sdbusplus::xyz::openbmc_project::Common::Error;
-    bool caughtException = false;
-    try
-    {
-        parser.getValues("Network", "abc");
-    }
-    catch (const std::exception& e)
-    {
-        caughtException = true;
-    }
-    EXPECT_EQ(true, caughtException);
+    config::ReturnCode rc = config::ReturnCode::SUCCESS;
+    config::ValueList values;
+    std::tie(rc, values) = parser.getValues("Network", "abc");
+    EXPECT_EQ(config::ReturnCode::KEY_NOT_FOUND, rc);
     remove("/tmp/eth0.network");
 }
 
 }//namespace network
 }//namespace phosphor
-
diff --git a/test/test_ethernet_interface.cpp b/test/test_ethernet_interface.cpp
index 27fbb67..2181006 100644
--- a/test/test_ethernet_interface.cpp
+++ b/test/test_ethernet_interface.cpp
@@ -197,7 +197,9 @@ TEST_F(TestEthernetInterface, addNameServers)
     fs::path filePath = confDir;
     filePath /= "00-bmc-test0.network";
     config::Parser parser(filePath.string());
-    auto values = parser.getValues("Network", "DNS");
+    config::ReturnCode rc = config::ReturnCode::SUCCESS;
+    config::ValueList values;
+    std::tie(rc, values) = parser.getValues("Network", "DNS");
     EXPECT_EQ(servers , values);
 
     validateResolvFile(values);
@@ -210,7 +212,9 @@ TEST_F(TestEthernetInterface, addNTPServers)
     fs::path filePath = confDir;
     filePath /= "00-bmc-test0.network";
     config::Parser parser(filePath.string());
-    auto values = parser.getValues("Network", "NTP");
+    config::ReturnCode rc = config::ReturnCode::SUCCESS;
+    config::ValueList values;
+    std::tie(rc, values) = parser.getValues("Network", "NTP");
     EXPECT_EQ(servers , values);
 }
 
diff --git a/test/test_vlan_interface.cpp b/test/test_vlan_interface.cpp
index 9711a1a..00384f1 100644
--- a/test/test_vlan_interface.cpp
+++ b/test/test_vlan_interface.cpp
@@ -123,17 +123,20 @@ TEST_F(TestVlanInterface, createVLAN)
     filePath /= "test0.50.netdev";
 
     config::Parser parser(filePath.string());
-    auto values = parser.getValues("NetDev", "Name");
+    config::ReturnCode rc = config::ReturnCode::SUCCESS;
+    config::ValueList values;
+
+    std::tie(rc, values) = parser.getValues("NetDev", "Name");
     std::string expectedValue = "test0.50";
     bool found = isValueFound(values, expectedValue);
     EXPECT_EQ(found, true);
 
-    values = parser.getValues("NetDev", "Kind");
+    std::tie(rc, values) = parser.getValues("NetDev", "Kind");
     expectedValue = "vlan";
     found = isValueFound(values, expectedValue);
     EXPECT_EQ(found, true);
 
-    values = parser.getValues("VLAN", "Id");
+    std::tie(rc, values) = parser.getValues("VLAN", "Id");
     expectedValue = "50";
     found = isValueFound(values, expectedValue);
     EXPECT_EQ(found, true);
@@ -163,17 +166,20 @@ TEST_F(TestVlanInterface, createMultipleVLAN)
     fs::path filePath = confDir;
     filePath /= "test0.50.netdev";
     config::Parser parser(filePath.string());
-    auto values = parser.getValues("NetDev", "Name");
+    config::ReturnCode rc = config::ReturnCode::SUCCESS;
+    config::ValueList values;
+
+    std::tie(rc, values) = parser.getValues("NetDev", "Name");
     std::string expectedValue = "test0.50";
     bool found = isValueFound(values, expectedValue);
     EXPECT_EQ(found, true);
 
-    values = parser.getValues("NetDev", "Kind");
+    std::tie(rc, values) = parser.getValues("NetDev", "Kind");
     expectedValue = "vlan";
     found = isValueFound(values, expectedValue);
     EXPECT_EQ(found, true);
 
-    values = parser.getValues("VLAN", "Id");
+    std::tie(rc, values) = parser.getValues("VLAN", "Id");
     expectedValue = "50";
     found = isValueFound(values, expectedValue);
     EXPECT_EQ(found, true);
@@ -181,12 +187,12 @@ TEST_F(TestVlanInterface, createMultipleVLAN)
     filePath = confDir;
     filePath /= "test0.60.netdev";
     parser.setFile(filePath.string());
-    values = parser.getValues("NetDev", "Name");
+    std::tie(rc, values) = parser.getValues("NetDev", "Name");
     expectedValue = "test0.60";
     found = isValueFound(values, expectedValue);
     EXPECT_EQ(found, true);
 
-    values = parser.getValues("VLAN", "Id");
+    std::tie(rc, values) = parser.getValues("VLAN", "Id");
     expectedValue = "60";
     found = isValueFound(values, expectedValue);
     EXPECT_EQ(found, true);
diff --git a/timer.cpp b/timer.cpp
index afd4b8d..7052775 100644
--- a/timer.cpp
+++ b/timer.cpp
@@ -72,7 +72,6 @@ int Timer::timeoutHandler(sd_event_source* eventSource,
         timer->userCallBack();
     }
 
-    log<level::INFO>("Timer expired");
     sd_event_source_set_enabled(eventSource, SD_EVENT_OFF);
     return 0;
 }
diff --git a/util.cpp b/util.cpp
index 5e606a7..4b234f8 100644
--- a/util.cpp
+++ b/util.cpp
@@ -411,19 +411,21 @@ bool getDHCPValue(const std::string& confDir, const std::string& intf)
                            systemd::config::networkFileSuffix;
     confPath /= fileName;
 
-    try
+    auto rc = config::ReturnCode::SUCCESS;
+    config::ValueList values;
+    config::Parser parser(confPath.string());
+
+    std::tie(rc, values) = parser.getValues("Network", "DHCP");
+    if (rc != config::ReturnCode::SUCCESS)
     {
-        config::Parser parser(confPath.string());
-        auto values = parser.getValues("Network", "DHCP");
-        // There will be only single value for DHCP key.
-        if (values[0] == "true")
-        {
-            dhcp = true;
-        }
+        log<level::DEBUG>("Unable to get the value for Network[DHCP]",
+                entry("RC=%d", rc));
+        return dhcp;
     }
-    catch (InternalFailure& e)
+    // There will be only single value for DHCP key.
+    if (values[0] == "true")
     {
-        log<level::INFO>("Exception occurred during getting of DHCP value");
+        dhcp = true;
     }
     return dhcp;
 }