PFW: Fixed bug on BitParameterTypes

BZ: 44148

The save and restore operation would not work correctly when BitParameters
of a BitParameterBlock were used in more than a single Domain.
This patch aims to fixup this bug and introduces a new class: BitwiseAreaConfiguration.

Change-Id: I0aaccd57cf1cce33400f94a8879565171d283425
Orig-Change-Id: I7107f7db9f01cfff3c38cbac606a8c1e9bca8b5e
Signed-off-by: Frédéric Boisnard <fredericx.boisnard@intel.com>
Reviewed-on: http://android.intel.com:8080/58363
Reviewed-by: Mendi, EduardoX <eduardox.mendi@intel.com>
Tested-by: Mendi, EduardoX <eduardox.mendi@intel.com>
Reviewed-by: buildbot <buildbot@intel.com>
Tested-by: buildbot <buildbot@intel.com>

14 files changed, 178 insertions, 33 deletions

diff --git a/parameter/Android.mk b/parameter/Android.mk
index a27ae4c..0fbd9b3 100644
--- a/parameter/Android.mk
+++ b/parameter/Android.mk
@@ -67,6 +67,7 @@ LOCAL_SRC_FILES:= \
         XmlParameterSerializingContext.cpp \
         XmlDomainSerializingContext.cpp \
         BinarySerializableElement.cpp \
+        BitwiseAreaConfiguration.cpp \
         BitParameterBlockType.cpp \
         BitParameterBlock.cpp \
         BitParameterType.cpp \
diff --git a/parameter/AreaConfiguration.cpp b/parameter/AreaConfiguration.cpp
index 2e8c63d..8d2368d 100644
--- a/parameter/AreaConfiguration.cpp
+++ b/parameter/AreaConfiguration.cpp
@@ -41,10 +41,17 @@ CAreaConfiguration::CAreaConfiguration(const CConfigurableElement* pConfigurable
     _blackboard.setSize(_pConfigurableElement->getFootPrint());
 }
 
+CAreaConfiguration::CAreaConfiguration(const CConfigurableElement* pConfigurableElement, const CSyncerSet* pSyncerSet, uint32_t uiSize)
+    : _pConfigurableElement(pConfigurableElement), _pSyncerSet(pSyncerSet), _bValid(false)
+{
+    // Size blackboard
+    _blackboard.setSize(uiSize);
+}
+
 // Save data from current
 void CAreaConfiguration::save(const CParameterBlackboard* pMainBlackboard)
 {
-    pMainBlackboard->saveTo(&_blackboard, _pConfigurableElement->getOffset());
+    copyFrom(pMainBlackboard, _pConfigurableElement->getOffset());
 }
 
 // Apply data to current
@@ -52,7 +59,7 @@ bool CAreaConfiguration::restore(CParameterBlackboard* pMainBlackboard, bool bSy
 {
     assert(_bValid);
 
-    pMainBlackboard->restoreFrom(&_blackboard, _pConfigurableElement->getOffset());
+    copyTo(pMainBlackboard, _pConfigurableElement->getOffset());
 
     // Synchronize if required
     if (bSync) {
@@ -131,23 +138,21 @@ const CConfigurableElement* CAreaConfiguration::getConfigurableElement() const
     return _pConfigurableElement;
 }
 
-// Configuration merging (we assume from element is descendant of this)
-void CAreaConfiguration::copyFromInner(const CAreaConfiguration* pFromAreaConfiguration)
+void CAreaConfiguration::copyToOuter(CAreaConfiguration* pToAreaConfiguration) const
 {
-    assert(pFromAreaConfiguration->getConfigurableElement()->isDescendantOf(_pConfigurableElement));
+    assert(_pConfigurableElement->isDescendantOf(pToAreaConfiguration->getConfigurableElement()));
 
-    _blackboard.restoreFrom(&pFromAreaConfiguration->_blackboard, pFromAreaConfiguration->getConfigurableElement()->getOffset() - _pConfigurableElement->getOffset());
+    copyTo(&pToAreaConfiguration->_blackboard, _pConfigurableElement->getOffset() - pToAreaConfiguration->getConfigurableElement()->getOffset());
 }
 
-// Configuration splitting
-void CAreaConfiguration::copyToInner(CAreaConfiguration* pToAreaConfiguration) const
+void CAreaConfiguration::copyFromOuter(const CAreaConfiguration* pFromAreaConfiguration)
 {
-    assert(pToAreaConfiguration->getConfigurableElement()->isDescendantOf(_pConfigurableElement));
+    assert(_pConfigurableElement->isDescendantOf(pFromAreaConfiguration->getConfigurableElement()));
 
-    _blackboard.saveTo(&pToAreaConfiguration->_blackboard, pToAreaConfiguration->getConfigurableElement()->getOffset() - _pConfigurableElement->getOffset());
+    copyFrom(&pFromAreaConfiguration->_blackboard, _pConfigurableElement->getOffset() - pFromAreaConfiguration->getConfigurableElement()->getOffset());
 
     // Inner becomes valid
-    pToAreaConfiguration->setValid(true);
+    setValid(true);
 }
 
 // Serialization
@@ -175,4 +180,14 @@ void CAreaConfiguration::setValid(bool bValid)
     _bValid = bValid;
 }
 
+// Blackboard copies
+void CAreaConfiguration::copyTo(CParameterBlackboard* pToBlackboard, uint32_t uiOffset) const
+{
+    pToBlackboard->restoreFrom(&_blackboard, uiOffset);
+}
+
+void CAreaConfiguration::copyFrom(const CParameterBlackboard* pFromBlackboard, uint32_t uiOffset)
+{
+   pFromBlackboard->saveTo(&_blackboard, uiOffset);
+}
 
diff --git a/parameter/AreaConfiguration.h b/parameter/AreaConfiguration.h
index b5e7e4c..6afed71 100644
--- a/parameter/AreaConfiguration.h
+++ b/parameter/AreaConfiguration.h
@@ -64,10 +64,10 @@ public:
     const CConfigurableElement* getConfigurableElement() const;
 
     // Configuration merging
-    void copyFromInner(const CAreaConfiguration* pFromAreaConfiguration);
+    virtual void copyToOuter(CAreaConfiguration* pToAreaConfiguration) const;
 
     // Configuration splitting
-    void copyToInner(CAreaConfiguration* pToAreaConfiguration) const;
+    virtual void copyFromOuter(const CAreaConfiguration* pFromAreaConfiguration);
 
     // XML configuration settings parsing/composing
     bool serializeXmlSettings(CXmlElement& xmlConfigurableElementSettingsElementContent, CConfigurationAccessContext& configurationAccessContext);
@@ -77,19 +77,29 @@ public:
 
     // Data size
     uint32_t getSize() const;
+
+protected:
+    CAreaConfiguration(const CConfigurableElement* pConfigurableElement, const CSyncerSet* pSyncerSet, uint32_t uiSize);
+
 private:
+    // Blackboard copies
+    virtual void copyTo(CParameterBlackboard* pToBlackboard, uint32_t uiOffset) const;
+    virtual void copyFrom(const CParameterBlackboard* pFromBlackboard, uint32_t uiOffset);
+
     // Store validity
     void setValid(bool bValid);
 
+protected:
     // Associated configurable element
     const CConfigurableElement* _pConfigurableElement;
 
-    // Syncer set (required for immediate synchronization)
-    const CSyncerSet* _pSyncerSet;
-
     // Configurable element settings
     CParameterBlackboard _blackboard;
 
+private:
+    // Syncer set (required for immediate synchronization)
+    const CSyncerSet* _pSyncerSet;
+
     // Area configuration validity (invalid area configurations can't be restored)
     bool _bValid;
 };
diff --git a/parameter/BitParameter.cpp b/parameter/BitParameter.cpp
index 0aa99d7..abf29f3 100644
--- a/parameter/BitParameter.cpp
+++ b/parameter/BitParameter.cpp
@@ -34,6 +34,7 @@
 #include "ConfigurationAccessContext.h"
 #include "ParameterBlackboard.h"
 #include "BitParameterBlock.h"
+#include "BitwiseAreaConfiguration.h"
 
 #define base CBaseParameter
 
@@ -56,7 +57,7 @@ uint32_t CBitParameter::getBelongingBlockSize() const
 // Instantiation, allocation
 uint32_t CBitParameter::getFootPrint() const
 {
-    // Allocation made on parent side
+    // Allocation done at parent level
     return 0;
 }
 
@@ -165,3 +166,16 @@ void CBitParameter::doGet(type& value, uint32_t uiOffset, CParameterAccessContex
     // Convert
     static_cast<const CBitParameterType*>(getTypeElement())->fromBlackboard(value, uiData, parameterAccessContext);
 }
+
+// AreaConfiguration creation
+CAreaConfiguration* CBitParameter::createAreaConfiguration(const CSyncerSet* pSyncerSet) const
+{
+    return new CBitwiseAreaConfiguration(this, pSyncerSet);
+}
+
+// Access from area configuration
+uint32_t CBitParameter::merge(uint32_t uiOriginData, uint32_t uiNewData) const
+{
+    // Convert
+    return static_cast<const CBitParameterType*>(getTypeElement())->merge(uiOriginData, uiNewData);
+}
diff --git a/parameter/BitParameter.h b/parameter/BitParameter.h
index 3391e27..d22fa89 100644
--- a/parameter/BitParameter.h
+++ b/parameter/BitParameter.h
@@ -51,10 +51,17 @@ public:
 
     // Integer Access
     virtual bool accessAsInteger(uint32_t& uiValue, bool bSet, CParameterAccessContext& parameterAccessContext) const;
-private:
+
+    // AreaConfiguration creation
+    virtual CAreaConfiguration* createAreaConfiguration(const CSyncerSet* pSyncerSet) const;
+
     // Size
     uint32_t getBelongingBlockSize() const;
 
+    // Access from area configuration
+    uint32_t merge(uint32_t uiOriginData, uint32_t uiNewData) const;
+private:
+
     // String Access
     virtual bool doSetValue(const string& strValue, uint32_t uiOffset, CParameterAccessContext& parameterAccessContext) const;
     virtual void doGetValue(string& strValue, uint32_t uiOffset, CParameterAccessContext& parameterAccessContext) const;
diff --git a/parameter/BitParameterType.cpp b/parameter/BitParameterType.cpp
index 9a09a59..9a9b06b 100644
--- a/parameter/BitParameterType.cpp
+++ b/parameter/BitParameterType.cpp
@@ -197,6 +197,12 @@ void CBitParameterType::fromBlackboard(uint32_t& uiUserValue, uint32_t uiValue,
     uiUserValue = (uiValue & getMask()) >> _uiBitPos;
 }
 
+// Access from area configuration
+uint32_t CBitParameterType::merge(uint32_t uiOriginData, uint32_t uiNewData) const
+{
+    return (uiOriginData & ~getMask()) | (uiNewData & getMask());
+}
+
 // Bit Size
 uint32_t CBitParameterType::getBitSize() const
 {
diff --git a/parameter/BitParameterType.h b/parameter/BitParameterType.h
index ebfec7e..dee018a 100644
--- a/parameter/BitParameterType.h
+++ b/parameter/BitParameterType.h
@@ -51,6 +51,8 @@ public:
     // Integer
     bool toBlackboard(uint32_t uiUserValue, uint32_t& uiValue, CParameterAccessContext& parameterAccessContext) const;
     void fromBlackboard(uint32_t& uiUserValue, uint32_t uiValue, CParameterAccessContext& parameterAccessContext) const;
+    // Access from area configuration
+    uint32_t merge(uint32_t uiOriginData, uint32_t uiNewData) const;
 
     // Bit Size
     uint32_t getBitSize() const;
diff --git a/parameter/BitwiseAreaConfiguration.cpp b/parameter/BitwiseAreaConfiguration.cpp
new file mode 100644
index 0000000..d3ea463
--- /dev/null
+++ b/parameter/BitwiseAreaConfiguration.cpp
@@ -0,0 +1,59 @@
+#include "BitwiseAreaConfiguration.h"
+#include "BitParameter.h"
+#include "Subsystem.h"
+
+#define base CAreaConfiguration
+
+CBitwiseAreaConfiguration::CBitwiseAreaConfiguration(const CConfigurableElement *pConfigurableElement, const CSyncerSet *pSyncerSet)
+    : base(pConfigurableElement, pSyncerSet, static_cast<const CBitParameter*>(pConfigurableElement)->getBelongingBlockSize()),
+      _bBigEndian(pConfigurableElement->getBelongingSubsystem()->isBigEndian())
+{
+}
+
+// Blackboard copies
+void CBitwiseAreaConfiguration::copyTo(CParameterBlackboard* pToBlackboard, uint32_t uiOffset) const
+{
+    // Beware this code works on little endian architectures only!
+    const CBitParameter* pBitParameter = static_cast<const CBitParameter*>(_pConfigurableElement);
+
+    uint32_t uiSrcData = 0;
+    uint32_t uiDstData = 0;
+
+    /// Read/modify/write
+
+    // Read dst blackboard
+    pToBlackboard->readInteger(&uiDstData, pBitParameter->getBelongingBlockSize(), uiOffset, _bBigEndian);
+
+    // Read src blackboard
+    _blackboard.readInteger(&uiSrcData, pBitParameter->getBelongingBlockSize(), 0, _bBigEndian);
+
+    // Convert
+    uiDstData = pBitParameter->merge(uiDstData, uiSrcData);
+
+    // Write dst blackboard
+    pToBlackboard->writeInteger(&uiDstData, pBitParameter->getBelongingBlockSize(), uiOffset, _bBigEndian);
+}
+
+void CBitwiseAreaConfiguration::copyFrom(const CParameterBlackboard* pFromBlackboard, uint32_t uiOffset)
+{
+    // Beware this code works on little endian architectures only!
+    const CBitParameter* pBitParameter = static_cast<const CBitParameter*>(_pConfigurableElement);
+
+    uint32_t uiSrcData = 0;
+    uint32_t uiDstData = 0;
+
+    /// Read/modify/write
+
+    // Read dst blackboard
+    _blackboard.readInteger(&uiDstData, pBitParameter->getBelongingBlockSize(), 0, _bBigEndian);
+
+    // Read src blackboard
+    pFromBlackboard->readInteger(&uiSrcData, pBitParameter->getBelongingBlockSize(), uiOffset, _bBigEndian);
+
+    // Convert
+    uiDstData = pBitParameter->merge(uiDstData, uiSrcData);
+
+    // Write dst blackboard
+    _blackboard.writeInteger(&uiDstData, pBitParameter->getBelongingBlockSize(), 0, _bBigEndian);
+}
+
diff --git a/parameter/BitwiseAreaConfiguration.h b/parameter/BitwiseAreaConfiguration.h
new file mode 100644
index 0000000..3b351b6
--- /dev/null
+++ b/parameter/BitwiseAreaConfiguration.h
@@ -0,0 +1,20 @@
+#pragma once
+
+#include "AreaConfiguration.h"
+
+class CBitParameter;
+
+class CBitwiseAreaConfiguration : public CAreaConfiguration
+{
+public:
+    CBitwiseAreaConfiguration(const CConfigurableElement* pConfigurableElement, const CSyncerSet* pSyncerSet);
+
+private:
+    // Blackboard copies
+    virtual void copyTo(CParameterBlackboard* pToBlackboard, uint32_t uiOffset) const;
+    virtual void copyFrom(const CParameterBlackboard* pFromBlackboard, uint32_t uiOffset);
+
+    // Endianness
+    bool _bBigEndian;
+};
+
diff --git a/parameter/ConfigurableDomain.cpp b/parameter/ConfigurableDomain.cpp
index 4c6541f..c72b0a8 100644
--- a/parameter/ConfigurableDomain.cpp
+++ b/parameter/ConfigurableDomain.cpp
@@ -356,14 +356,7 @@ bool CConfigurableDomain::addConfigurableElement(CConfigurableElement* pConfigur
     log("Adding configurable element \"%s\" into domain \"%s\"", pConfigurableElement->getPath().c_str(), getName().c_str());
 
     // Do add
-    doAddConfigurableElement(pConfigurableElement);
-
-    // Ensure area validity for that configurable element (if main blackboard provided)
-    if (pMainBlackboard) {
-
-        // Need to validate against main blackboard
-        validateAreas(pConfigurableElement, pMainBlackboard);
-    }
+    doAddConfigurableElement(pConfigurableElement, pMainBlackboard);
 
     return true;
 }
@@ -931,7 +924,7 @@ void CConfigurableDomain::mergeConfigurations(CConfigurableElement* pToConfigura
 }
 
 // Configurable elements association
-void CConfigurableDomain::doAddConfigurableElement(CConfigurableElement* pConfigurableElement)
+void CConfigurableDomain::doAddConfigurableElement(CConfigurableElement* pConfigurableElement, const CParameterBlackboard *pMainBlackboard)
 {
     // Inform configurable element
     pConfigurableElement->addAttachedConfigurableDomain(this);
@@ -959,7 +952,14 @@ void CConfigurableDomain::doAddConfigurableElement(CConfigurableElement* pConfig
         pDomainConfiguration->addConfigurableElement(pConfigurableElement, pSyncerSet);
     }
 
-    // Already associated descended configurable elements need a merge of their configuration data
+    // Ensure area validity for that configurable element (if main blackboard provided)
+    if (pMainBlackboard) {
+
+        // Need to validate against main blackboard
+        validateAreas(pConfigurableElement, pMainBlackboard);
+    }
+
+    // Already associated descendend configurable elements need a merge of their configuration data
     mergeAlreadyAssociatedDescendantConfigurableElements(pConfigurableElement);
 
     // Add to list
diff --git a/parameter/ConfigurableDomain.h b/parameter/ConfigurableDomain.h
index ac3c5c9..542bb52 100644
--- a/parameter/ConfigurableDomain.h
+++ b/parameter/ConfigurableDomain.h
@@ -132,7 +132,7 @@ private:
     void mergeConfigurations(CConfigurableElement* pToConfigurableElement, CConfigurableElement* pFromConfigurableElement);
 
     // Configurable elements association
-    void doAddConfigurableElement(CConfigurableElement* pConfigurableElement);
+    void doAddConfigurableElement(CConfigurableElement* pConfigurableElement, const CParameterBlackboard* pMainBlackboard = NULL);
     void doRemoveConfigurableElement(CConfigurableElement* pConfigurableElement, bool bRecomputeSyncSet);
 
     // XML parsing
diff --git a/parameter/ConfigurableElement.cpp b/parameter/ConfigurableElement.cpp
index 93acf3f..910b6dc 100644
--- a/parameter/ConfigurableElement.cpp
+++ b/parameter/ConfigurableElement.cpp
@@ -34,6 +34,7 @@
 #include "ConfigurableDomain.h"
 #include "ConfigurationAccessContext.h"
 #include "ConfigurableElementAggregator.h"
+#include "AreaConfiguration.h"
 #include <assert.h>
 
 #define base CElement
@@ -126,6 +127,12 @@ bool CConfigurableElement::serializeXmlSettings(CXmlElement& xmlConfigurationSet
     return true;
 }
 
+// AreaConfiguration creation
+CAreaConfiguration* CConfigurableElement::createAreaConfiguration(const CSyncerSet* pSyncerSet) const
+{
+    return new CAreaConfiguration(this, pSyncerSet);
+}
+
 // Parameter access
 bool CConfigurableElement::accessValue(CPathNavigator& pathNavigator, string& strValue, bool bSet, CParameterAccessContext& parameterAccessContext) const
 {
@@ -225,7 +232,7 @@ ISyncer* CConfigurableElement::getSyncer() const
 
         return static_cast<const CConfigurableElement*>(pParent)->getSyncer();
     }
-    return false;
+    return NULL;
 }
 
 // Syncer set (me, ascendant or descendant ones)
diff --git a/parameter/ConfigurableElement.h b/parameter/ConfigurableElement.h
index 41e33bf..4f5dfc5 100644
--- a/parameter/ConfigurableElement.h
+++ b/parameter/ConfigurableElement.h
@@ -40,6 +40,7 @@ class ISyncer;
 class CSubsystem;
 class CConfigurationAccessContext;
 class CParameterAccessContext;
+class CAreaConfiguration;
 
 class CConfigurableElement : public CElement
 {
@@ -91,6 +92,9 @@ public:
     // Check element is a parameter
     virtual bool isParameter() const;
 
+    // AreaConfiguration creation
+    virtual CAreaConfiguration* createAreaConfiguration(const CSyncerSet* pSyncerSet) const;
+
     // Parameter access
     virtual bool accessValue(CPathNavigator& pathNavigator, string& strValue, bool bSet, CParameterAccessContext& parameterAccessContext) const;
     // Used for simulation and virtual subsystems
diff --git a/parameter/DomainConfiguration.cpp b/parameter/DomainConfiguration.cpp
index f746be9..9e341f2 100644
--- a/parameter/DomainConfiguration.cpp
+++ b/parameter/DomainConfiguration.cpp
@@ -212,7 +212,7 @@ bool CDomainConfiguration::serializeConfigurableElementSettings(CAreaConfigurati
 // Configurable Elements association
 void CDomainConfiguration::addConfigurableElement(const CConfigurableElement* pConfigurableElement, const CSyncerSet* pSyncerSet)
 {
-    CAreaConfiguration* pAreaConfiguration = new CAreaConfiguration(pConfigurableElement, pSyncerSet);
+    CAreaConfiguration* pAreaConfiguration = pConfigurableElement->createAreaConfiguration(pSyncerSet);
 
     _areaConfigurationList.push_back(pAreaConfiguration);
     _orderedAreaConfigurationList.push_back(pAreaConfiguration);
@@ -433,7 +433,7 @@ void CDomainConfiguration::merge(CConfigurableElement* pToConfigurableElement, C
     const CAreaConfiguration* pAreaConfigurationToMergeFrom = getAreaConfiguration(pFromConfigurableElement);
 
     // Do the merge
-    pAreaConfigurationToMergeTo->copyFromInner(pAreaConfigurationToMergeFrom);
+    pAreaConfigurationToMergeFrom->copyToOuter(pAreaConfigurationToMergeTo);
 }
 
 // Domain splitting
@@ -454,7 +454,7 @@ void CDomainConfiguration::split(CConfigurableElement* pFromConfigurableElement)
         CAreaConfiguration* pChildAreaConfiguration = getAreaConfiguration(pToChildConfigurableElement);
 
         // Do the copy
-        pAreaConfigurationToSplitFrom->copyToInner(pChildAreaConfiguration);
+        pChildAreaConfiguration->copyFromOuter(pAreaConfigurationToSplitFrom);
     }
 }