Enhance testability of TemplateManager class 27/235127/12
authorTomasz Swierczek <t.swierczek@samsung.com>
Tue, 2 Jun 2020 07:00:17 +0000 (09:00 +0200)
committerTomasz Swierczek <t.swierczek@samsung.com>
Wed, 24 Jun 2020 12:19:06 +0000 (14:19 +0200)
The getAllMappedPrivs() method used to have a static variable
holding mapped privileges - the configuration was meant to be loaded
only once to improve performance, effectively by limiting runtime
allocation of a std::vector<std::string>.

However, the class holds other data in instance variables, that can
be filled at init() call on each object creation. This can cause
inconsistency that make ie. the test T1138_all_mapped_privileges
to fail because of different configuration loaded vs. stored in static
variable.

This commit removes the static variable, calculating the instance-level
variable on init() instead - this allows various configurations to be
tested in single unit test framework binary and keeps the performance
optimization, while wasting some memory.

Change-Id: Ic18bf1ca34e4a8deba2e0d876a735c29a277f4f6

src/common/include/template-manager.h
src/common/template-manager.cpp

index 13440e8e99548b6b3eea359dbe1c3111bde7c42a..75827d7f22f897a647276cbad0832201a5ccfdba 100644 (file)
@@ -68,11 +68,11 @@ private:
     std::string getPolicyFile(enum TemplateManager::Type policyFile,
                               const std::string &privFile = "") const ;
     PrivMapping getPrivMapping(const std::string &privName) const;
-    std::vector<std::string> getPrivMappingMapKeys() const;
 
     std::string m_rootDir;
     std::map<Path, Smack::TemplateRules> m_templateRules;
     std::map<PrivilegeName, PrivMapping> m_privMapping;
+    std::vector<std::string> m_privs;
 };
 
 } // namespace SecurityManager
index 9aa0f39710949d7a926e68bbd3b32ce179031c32..d472ff5f08ad181a63ba9582b20d34db6524101d 100644 (file)
@@ -113,6 +113,12 @@ void TemplateManager::loadFiles()
         auto path = getPolicyFile(PRIV_RULES_TEMPLATE, privMapping.second.fileName);
         m_templateRules[path] = Smack::fromConfig(path);
     }
+
+    m_privs.resize(m_privMapping.size());
+    size_t i = 0;
+    for (auto &mapping : m_privMapping) {
+        m_privs[i++] = mapping.first;
+    }
 }
 
 bool TemplateManager::isPrivilegeMappingEnabled() const
@@ -120,20 +126,9 @@ bool TemplateManager::isPrivilegeMappingEnabled() const
     return !m_privMapping.empty();
 }
 
-std::vector<std::string> TemplateManager::getPrivMappingMapKeys() const
-{
-    std::vector<std::string> keys;
-    keys.reserve(m_privMapping.size());
-    for (auto &mapping : m_privMapping) {
-        keys.push_back(mapping.first);
-    }
-    return keys;
-}
-
 const std::vector<std::string>& TemplateManager::getAllMappedPrivs() const
 {
-    static const std::vector<std::string> allMappedPrivs = getPrivMappingMapKeys();
-    return allMappedPrivs;
+    return m_privs;
 }
 
 TemplateManager::PrivMapping