commit fda15a33f2f1f20b214f9893455fa24696ea834b
author Lei YU <mine260309@gmail.com> Thu Sep 19 14:43:02 2019 +0800
committer Lei YU <mine260309@gmail.com> Wed Oct 23 06:53:42 2019 +0000
tree 4e92beb8885e01a8e914f4a49f1728bc5f91d4f9
parent 7f2a2152cf5c9bff7e0e6e8d2cc399c3a5033300

Refactor Version::getValue()

Rename the function to getValues() and change the parameter and return
value types, and add a test case for it.
It will be used in future commits.

Tested: Verify the unit test case passes.

Signed-off-by: Lei YU <mine260309@gmail.com>
Change-Id: I57ccf857737ef13f4e2f27c5f2fb7400a2170e91

src/item_updater.cpp

diff --git a/src/item_updater.cpp b/src/item_updater.cpp
index e100381..d9a502f 100644
--- a/src/item_updater.cpp
+++ b/src/item_updater.cpp
@@ -9,6 +9,11 @@
 #include <phosphor-logging/log.hpp>
 #include <xyz/openbmc_project/Common/error.hpp>
 
+namespace
+{
+constexpr auto EXTENDED_VERSION = "extended_version";
+}
+
 namespace phosphor
 {
 namespace software
@@ -98,12 +103,14 @@
 
         fs::path manifestPath(filePath);
         manifestPath /= MANIFEST_FILE;
-        std::string extendedVersion =
-            (Version::getValue(
-                 manifestPath.string(),
-                 std::map<std::string, std::string>{{"extended_version", ""}}))
-                .begin()
-                ->second;
+        std::string extendedVersion;
+        auto values =
+            Version::getValues(manifestPath.string(), {EXTENDED_VERSION});
+        const auto it = values.find(EXTENDED_VERSION);
+        if (it != values.end())
+        {
+            extendedVersion = it->second;
+        }
 
         auto activation = createActivationObject(
             path, versionId, extendedVersion, activationState, associations);

src/version.cpp

diff --git a/src/version.cpp b/src/version.cpp
index 7940d2e..0ea6cc9 100644
--- a/src/version.cpp
+++ b/src/version.cpp
@@ -23,8 +23,8 @@
 using Argument = xyz::openbmc_project::Common::InvalidArgument;
 
 std::map<std::string, std::string>
-    Version::getValue(const std::string& filePath,
-                      std::map<std::string, std::string> keys)
+    Version::getValues(const std::string& filePath,
+                       const std::vector<std::string>& keys)
 {
     if (filePath.empty())
     {
@@ -33,39 +33,24 @@
                               Argument::ARGUMENT_VALUE(filePath.c_str()));
     }
 
-    std::ifstream efile;
+    std::ifstream efile(filePath);
     std::string line;
-    efile.exceptions(std::ifstream::failbit | std::ifstream::badbit |
-                     std::ifstream::eofbit);
+    std::map<std::string, std::string> ret;
 
-    try
+    while (getline(efile, line))
     {
-        efile.open(filePath);
-        while (getline(efile, line))
+        for (const auto& key : keys)
         {
-            for (auto& key : keys)
+            auto value = key + "=";
+            auto keySize = value.length();
+            if (line.compare(0, keySize, value) == 0)
             {
-                auto value = key.first + "=";
-                auto keySize = value.length();
-                if (line.compare(0, keySize, value) == 0)
-                {
-                    key.second = line.substr(keySize);
-                    break;
-                }
+                ret.emplace(key, line.substr(keySize));
+                break;
             }
         }
-        efile.close();
     }
-    catch (const std::exception& e)
-    {
-        if (!efile.eof())
-        {
-            log<level::ERR>("Error in reading file");
-        }
-        efile.close();
-    }
-
-    return keys;
+    return ret;
 }
 
 void Delete::delete_()

src/version.hpp

diff --git a/src/version.hpp b/src/version.hpp
index a67fc82..c3c2c54 100644
--- a/src/version.hpp
+++ b/src/version.hpp
@@ -121,13 +121,13 @@
      *
      * @param[in] filePath - The path to the file which contains the value
      *                       of keys.
-     * @param[in] keys     - A map of keys with empty values.
+     * @param[in] keys     - A vector of keys.
      *
      * @return The map of keys with filled values.
      **/
     static std::map<std::string, std::string>
-        getValue(const std::string& filePath,
-                 std::map<std::string, std::string> keys);
+        getValues(const std::string& filePath,
+                  const std::vector<std::string>& keys);
 
     /** @brief The temUpdater's erase callback. */
     eraseFunc eraseCallback;