From e5c4f1d7a751a6ded6bb099d1949c3871d9c487a Mon Sep 17 00:00:00 2001
From: Johnathan Mantey <johnathanx.mantey@intel.com>
Date: Mon, 10 Dec 2018 16:24:26 -0800
Subject: Eliminate public function returning pointer to private class data

The getChannelDataPtr method breaks class encapsulation.  Only class
methods are supposed to have access to class private instance variables.

Change-Id: I5dbfb75f0fa409b82a1e7f426b2034d39f7df9ad
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
---
 user_channel/channel_mgmt.cpp | 45 +++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 27 deletions(-)

(limited to 'user_channel/channel_mgmt.cpp')

diff --git a/user_channel/channel_mgmt.cpp b/user_channel/channel_mgmt.cpp
index 697219c..859045a 100644
--- a/user_channel/channel_mgmt.cpp
+++ b/user_channel/channel_mgmt.cpp
@@ -151,8 +151,8 @@ std::string getNetIntfFromPath(const std::string& path)
     return intfName;
 }
 
-void processChAccessPropChange(ChannelConfig& chConfig, const std::string& path,
-                               const DbusChObjProperties& chProperties)
+void ChannelConfig::processChAccessPropChange(
+    const std::string& path, const DbusChObjProperties& chProperties)
 {
     // Get interface name from path. ex: '/xyz/openbmc_project/network/eth0'
     std::string channelName;
@@ -195,8 +195,7 @@ void processChAccessPropChange(ChannelConfig& chConfig, const std::string& path,
     uint8_t intfPriv = 0;
     try
     {
-        intfPriv =
-            static_cast<uint8_t>(chConfig.convertToPrivLimitIndex(intfPrivStr));
+        intfPriv = static_cast<uint8_t>(convertToPrivLimitIndex(intfPrivStr));
     }
     catch (const std::invalid_argument& e)
     {
@@ -205,14 +204,12 @@ void processChAccessPropChange(ChannelConfig& chConfig, const std::string& path,
     }
 
     boost::interprocess::scoped_lock<boost::interprocess::named_recursive_mutex>
-        channelLock{*chConfig.channelMutex};
+        channelLock{*channelMutex};
     uint8_t chNum = 0;
-    ChannelData* chData;
     // Get the channel number based on the channel name.
     for (chNum = 0; chNum < maxIpmiChannels; chNum++)
     {
-        chData = chConfig.getChannelDataPtr(chNum);
-        if (chData->chName == channelName)
+        if (channelData[chNum].chName == channelName)
         {
             break;
         }
@@ -224,9 +221,9 @@ void processChAccessPropChange(ChannelConfig& chConfig, const std::string& path,
     }
 
     // skip updating the values, if this property change originated from IPMI.
-    if (chConfig.signalFlag & (1 << chNum))
+    if (signalFlag & (1 << chNum))
     {
-        chConfig.signalFlag &= ~(1 << chNum);
+        signalFlag &= ~(1 << chNum);
         log<level::DEBUG>("Request originated from IPMI so ignoring signal");
         return;
     }
@@ -234,21 +231,23 @@ void processChAccessPropChange(ChannelConfig& chConfig, const std::string& path,
     // Update both volatile & Non-volatile, if there is mismatch.
     // as property change other than IPMI, has to update both volatile &
     // non-volatile data.
-    if (chData->chAccess.chNonVolatileData.privLimit != intfPriv)
+    checkAndReloadVolatileData();
+    checkAndReloadNVData();
+    if (channelData[chNum].chAccess.chNonVolatileData.privLimit != intfPriv)
     {
         // Update NV data
-        chData->chAccess.chNonVolatileData.privLimit = intfPriv;
-        if (chConfig.writeChannelPersistData() != 0)
+        channelData[chNum].chAccess.chNonVolatileData.privLimit = intfPriv;
+        if (writeChannelPersistData() != 0)
         {
             log<level::ERR>("Failed to update the persist data file");
             return;
         }
 
         // Update Volatile data
-        if (chData->chAccess.chVolatileData.privLimit != intfPriv)
+        if (channelData[chNum].chAccess.chVolatileData.privLimit != intfPriv)
         {
-            chData->chAccess.chVolatileData.privLimit = intfPriv;
-            if (chConfig.writeChannelVolatileData() != 0)
+            channelData[chNum].chAccess.chVolatileData.privLimit = intfPriv;
+            if (writeChannelVolatileData() != 0)
             {
                 log<level::ERR>("Failed to update the volatile data file");
                 return;
@@ -325,19 +324,12 @@ ChannelConfig::ChannelConfig() : bus(ipmid_get_sd_bus_connection())
                 std::string iface;
                 std::string path = msg.get_path();
                 msg.read(iface, props);
-                processChAccessPropChange(*this, path, props);
+                processChAccessPropChange(path, props);
             });
         signalHndlrObjectState = true;
     }
 }
 
-ChannelData* ChannelConfig::getChannelDataPtr(const uint8_t chNum)
-{
-    // reload data before using it.
-    checkAndReloadVolatileData();
-    return &channelData[chNum];
-}
-
 bool ChannelConfig::isValidChannel(const uint8_t chNum)
 {
     if (chNum > maxIpmiChannels)
@@ -349,10 +341,9 @@ bool ChannelConfig::isValidChannel(const uint8_t chNum)
     if (channelData[chNum].isChValid == false)
     {
         log<level::DEBUG>("Channel is not valid");
-        return false;
     }
 
-    return true;
+    return channelData[chNum].isChValid;
 }
 
 EChannelSessSupported
@@ -879,7 +870,7 @@ int ChannelConfig::loadChannelConfig()
         channelLock{*channelMutex};
 
     Json data = readJsonFile(channelConfigDefaultFilename);
-    if (data == nullptr)
+    if (data.empty())
     {
         log<level::DEBUG>("Error in opening IPMI Channel data file");
         return -EIO;
-- 
cgit v1.2.1