Have the Target class manage the file descriptor

The sysfs file descriptor will now stay open from first
use until the Target class is destroyed.

Change-Id: Ib9803a527115cc75ecc9a5351d1e748ccd54a993
Signed-off-by: Matt Spinler <spinler@us.ibm.com>

7 files changed, 42 insertions, 21 deletions

diff --git a/cfam_access.cpp b/cfam_access.cpp
index 2762e80..888eefc 100644
--- a/cfam_access.cpp
+++ b/cfam_access.cpp
@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 #include <unistd.h>
-#include "filedescriptor.hpp"
 #include "cfam_access.hpp"
 #include "targeting.hpp"
 
@@ -28,7 +27,7 @@ namespace access
 constexpr auto cfamRegSize = 4;
 
 using namespace openpower::targeting;
-using namespace openpower::p9_util;
+using namespace openpower::util;
 
 /**
  * Converts the CFAM register address used by the calling
@@ -45,9 +44,7 @@ void writeReg(const std::unique_ptr<Target>& target,
               cfam_address_t address,
               cfam_data_t data)
 {
-    FileDescriptor fd(target->getPath());
-
-    int rc = lseek(fd(), makeOffset(address), SEEK_SET);
+    int rc = lseek(target->getCFAMFD(), makeOffset(address), SEEK_SET);
     if (rc < 0)
     {
         //Future: use a different exception to create an error log
@@ -58,7 +55,7 @@ void writeReg(const std::unique_ptr<Target>& target,
         throw std::runtime_error(msg);
     }
 
-    rc = write(fd(), &data, cfamRegSize);
+    rc = write(target->getCFAMFD(), &data, cfamRegSize);
     if (rc < 0)
     {
         //Future: use a different exception to create an error log
@@ -74,10 +71,9 @@ void writeReg(const std::unique_ptr<Target>& target,
 cfam_data_t readReg(const std::unique_ptr<Target>& target,
                     cfam_address_t address)
 {
-    FileDescriptor fd(target->getPath());
     cfam_data_t data = 0;
 
-    int rc = lseek(fd(), makeOffset(address), SEEK_SET);
+    int rc = lseek(target->getCFAMFD(), makeOffset(address), SEEK_SET);
     if (rc < 0)
     {
         //Future: use a different exception to create an error log
@@ -88,7 +84,7 @@ cfam_data_t readReg(const std::unique_ptr<Target>& target,
         throw std::runtime_error(msg);
     }
 
-    rc = read(fd(), &data, cfamRegSize);
+    rc = read(target->getCFAMFD(), &data, cfamRegSize);
     if (rc < 0)
     {
         //Future: use a different exception to create an error log
diff --git a/filedescriptor.cpp b/filedescriptor.cpp
index 3b1b096..c8bcab0 100644
--- a/filedescriptor.cpp
+++ b/filedescriptor.cpp
@@ -19,7 +19,7 @@
 
 namespace openpower
 {
-namespace p9_util
+namespace util
 {
 
 FileDescriptor::FileDescriptor(const std::string& path)
diff --git a/filedescriptor.hpp b/filedescriptor.hpp
index b03d314..2a4c6d0 100644
--- a/filedescriptor.hpp
+++ b/filedescriptor.hpp
@@ -5,7 +5,7 @@
 
 namespace openpower
 {
-namespace p9_util
+namespace util
 {
 
 /**
@@ -40,7 +40,7 @@ class FileDescriptor
         /**
          * The method to access the file descriptor value
          */
-        inline int operator()() const
+        inline auto get() const
         {
             return fd;
         }
diff --git a/targeting.cpp b/targeting.cpp
index 11c359c..e7d8b7a 100644
--- a/targeting.cpp
+++ b/targeting.cpp
@@ -26,6 +26,18 @@ namespace targeting
 using namespace phosphor::logging;
 namespace fs = std::experimental::filesystem;
 
+int Target::getCFAMFD()
+{
+    if (cfamFD.get() == nullptr)
+    {
+        cfamFD = std::make_unique<
+            openpower::util::FileDescriptor>(getCFAMPath());
+    }
+
+    return cfamFD->get();
+}
+
+
 Targeting::Targeting(const std::string& fsiMasterDev,
                      const std::string& fsiSlaveDir) :
     fsiMasterPath(fsiMasterDev),
diff --git a/targeting.hpp b/targeting.hpp
index 92c224e..463ccc5 100644
--- a/targeting.hpp
+++ b/targeting.hpp
@@ -2,6 +2,7 @@
 
 #include <memory>
 #include <vector>
+#include "filedescriptor.hpp"
 
 namespace openpower
 {
@@ -28,7 +29,7 @@ class Target
          * @param[in] - The sysfs device path
          */
         Target(size_t position, const std::string& devPath) :
-            pos(position), path(devPath)
+            pos(position), cfamPath(devPath)
         {
         }
 
@@ -47,13 +48,19 @@ class Target
         }
 
         /**
-         * Returns the path
+         * Returns the CFAM sysfs path
          */
-        inline auto getPath() const
+        inline auto getCFAMPath() const
         {
-            return path;
+            return cfamPath;
         }
 
+        /**
+         * Returns the file descriptor to use
+         * for read/writeCFAM operations.
+         */
+        int getCFAMFD();
+
     private:
 
         /**
@@ -62,9 +69,14 @@ class Target
         size_t pos;
 
         /**
-         * The sysfs device path
+         * The sysfs device path for the CFAM
+         */
+        const std::string cfamPath;
+
+        /**
+         * The file descriptor to use for read/writeCFAMReg
          */
-        const std::string path;
+        std::unique_ptr<openpower::util::FileDescriptor> cfamFD;
 };
 
 
diff --git a/test/Makefile.am b/test/Makefile.am
index bf86e6a..e0d20a2 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -13,4 +13,5 @@ utest_LDFLAGS = -lgtest_main -lgtest $(PTHREAD_LIBS) $(OESDK_TESTCASE_FLAGS) \
 	$(PHOSPHOR_LOGGING_LIBS) -lstdc++fs
 
 utest_SOURCES = utest.cpp
-utest_LDADD = $(top_srcdir)/targeting.cpp $(top_srcdir)/registration.cpp
+utest_LDADD = $(top_srcdir)/targeting.cpp $(top_srcdir)/registration.cpp \
+	$(top_srcdir)/filedescriptor.cpp
diff --git a/test/utest.cpp b/test/utest.cpp
index 0d3c2c9..4572c55 100644
--- a/test/utest.cpp
+++ b/test/utest.cpp
@@ -62,7 +62,7 @@ TEST_F(TargetingTest, CreateTargets)
         auto t = targets.begin();
         ASSERT_EQ((*t)->getPos(), 0);
 
-        ASSERT_EQ((*t)->getPath(), masterDir);
+        ASSERT_EQ((*t)->getCFAMPath(), masterDir);
     }
 
 
@@ -95,7 +95,7 @@ TEST_F(TargetingTest, CreateTargets)
                 path << _directory << "/slave@0" << i << ":00/raw";
             }
 
-            ASSERT_EQ(t->getPath(), path.str());
+            ASSERT_EQ(t->getCFAMPath(), path.str());
             i++;
         }
     }