parameter-framework: import/export XML

BZ: 10948

- Max value handling on integers corrected
- Left-justified Qn.m numbers
- Corrections after code review: removed fixed numbers from the code and unified byte to bit conversions

Change-Id: Iaf54e413201eae61013735580e046c5ab1874700
Signed-off-by: Patrick Benavoli <patrickx.benavoli@intel.com>
Reviewed-on: http://android.intel.com:8080/22316
Reviewed-by: Centelles, Sylvain <sylvain.centelles@intel.com>
Tested-by: Barthes, FabienX <fabienx.barthes@intel.com>
Reviewed-by: buildbot <buildbot@intel.com>
Tested-by: buildbot <buildbot@intel.com>
Reviewed-on: http://android.intel.com:8080/26777
Reviewed-by: Barthes, FabienX <fabienx.barthes@intel.com>

5 files changed, 40 insertions, 53 deletions

diff --git a/parameter/BitParameterType.cpp b/parameter/BitParameterType.cpp
index d4833e3..31d1d77 100644
--- a/parameter/BitParameterType.cpp
+++ b/parameter/BitParameterType.cpp
@@ -72,10 +72,10 @@ bool CBitParameterType::fromXml(const CXmlElement& xmlElement, CXmlSerializingCo
     // Size
     _uiBitSize = xmlElement.getAttributeInteger("Size");
 
-    // TODO: Validate bit pos and size still fit into parent type
+    // Validate bit pos and size still fit into parent type
     const CBitParameterBlockType* pBitParameterBlockType = static_cast<const CBitParameterBlockType*>(getParent());
 
-    uint32_t uiParentBlockBitSize = pBitParameterBlockType->getSize() << 3;
+    uint32_t uiParentBlockBitSize = pBitParameterBlockType->getSize() * 8;
 
     if (_uiBitPos + _uiBitSize > uiParentBlockBitSize) {
 
@@ -173,7 +173,7 @@ uint32_t CBitParameterType::getMask() const
 // Check data has no bit set outside available range
 bool CBitParameterType::isEncodable(uint32_t uiData) const
 {
-    uint32_t uiShift = 32 - _uiBitSize;
+    uint32_t uiShift = 8 * sizeof(uiData) - _uiBitSize;
 
     if (uiShift) {
 
diff --git a/parameter/FixedPointParameterType.cpp b/parameter/FixedPointParameterType.cpp
index 5b90f13..0c1cc07 100644
--- a/parameter/FixedPointParameterType.cpp
+++ b/parameter/FixedPointParameterType.cpp
@@ -128,19 +128,10 @@ bool CFixedPointParameterType::asInteger(const string& strValue, uint32_t& uiVal
         // Get data in integer form
         iData = strtol(strValue.c_str(), NULL, 0);
 
-        if (bValueProvidedAsHexa) {
+        if (bValueProvidedAsHexa && isEncodable(iData)) {
 
-            if (!isEncodable(iData, getUtilSizeInBits())) {
-
-                // Illegal value provided
-                parameterAccessContext.setError(getOutOfRangeError(strValue, parameterAccessContext.valueSpaceIsRaw(), true));
-
-                return false;
-            } else {
-
-                // Sign extend
-                signExtend(iData);
-            }
+            // Sign extend
+            signExtend(iData);
         }
 
     } else {
@@ -148,6 +139,8 @@ bool CFixedPointParameterType::asInteger(const string& strValue, uint32_t& uiVal
 
         // Do the conversion
         iData = (int32_t)(dData * (1UL << _uiFractional) + 0.5F - (double)(dData < 0));
+        // Left justify
+        iData <<= getSize() * 8 - getUtilSizeInBits();
     }
 
     // Check integrity
@@ -159,7 +152,7 @@ bool CFixedPointParameterType::asInteger(const string& strValue, uint32_t& uiVal
         return false;
     }
 
-    uiValue = (uint32_t)iData;
+    uiValue = iData;
 
     return true;
 }
@@ -169,7 +162,7 @@ void CFixedPointParameterType::asString(const uint32_t& uiValue, string& strValu
     int32_t iData = uiValue;
 
     // Check consistency
-    assert(isEncodable(iData, getUtilSizeInBits()));
+    assert(isEncodable(iData));
 
     // Sign extend
     signExtend(iData);
@@ -183,13 +176,16 @@ void CFixedPointParameterType::asString(const uint32_t& uiValue, string& strValu
         // Hexa formatting?
         if (parameterAccessContext.outputRawFormatIsHex()) {
 
-            strStream << "0x" << hex << uppercase << setw(getSize()*2) << setfill('0') << uiValue;
+            strStream << "0x" << hex << uppercase << setw(getSize()*2) << setfill('0') << (uint32_t)iData;
         } else {
 
             strStream << iData;
         }
     } else {
 
+        // Unjustify
+        iData >>= getSize() * 8 - getUtilSizeInBits();
+
         double dData = (double)iData / (1UL << _uiFractional);
 
         strStream << dData;
@@ -208,7 +204,7 @@ uint32_t CFixedPointParameterType::getUtilSizeInBits() const
 string CFixedPointParameterType::getOutOfRangeError(const string& strValue, bool bRawValueSpace, bool bHexaValue) const
 {
     // Min/Max computation
-    int32_t iMax = (1L << (getUtilSizeInBits() - 1)) - 1;
+    int32_t iMax = (1L << (getSize() * 8 - 1)) - 1;
     int32_t iMin = -iMax - 1;
 
     ostringstream strStream;
@@ -244,17 +240,17 @@ string CFixedPointParameterType::getOutOfRangeError(const string& strValue, bool
 // Check data is consistent with available range, with respect to its sign
 bool CFixedPointParameterType::isConsistent(uint32_t uiData) const
 {
-    uint32_t uiShift = 32 - getUtilSizeInBits();
+    uint32_t uiShift = getSize() * 8;
 
-    if (uiShift) {
-
-        // Negative value?
-        bool bIsValueExpectedNegative = (uiData & (1 << (uiShift - 1))) != 0;
-
-        // Check high bits are clean
-        return bIsValueExpectedNegative ? !(~uiData >> uiShift) : !(uiData >> uiShift);
+    if (uiShift == 8 * sizeof(uiData)) {
+        // Prevent inappropriate shifts
+        return true;
     }
 
-    return true;
+    // Negative value?
+    bool bIsValueExpectedNegative = (uiData & (1 << (uiShift - 1))) != 0;
+
+    // Check high bits are clean
+    return bIsValueExpectedNegative ? !(~uiData >> uiShift) : !(uiData >> uiShift);
 }
 
diff --git a/parameter/IntegerParameterType.cpp b/parameter/IntegerParameterType.cpp
index f3e447e..a55adff 100644
--- a/parameter/IntegerParameterType.cpp
+++ b/parameter/IntegerParameterType.cpp
@@ -71,24 +71,24 @@ bool CIntegerParameterType::fromXml(const CXmlElement& xmlElement, CXmlSerializi
     // Sign
     _bSigned = xmlElement.getAttributeBoolean("Signed");
 
-    // Size
-    setSize(xmlElement.getAttributeInteger("Size") / 8);
-
     // Size in bits
-    uint32_t uiUtilSizeInBits = getSize() << 3;
+    uint32_t uiSizeInBits = xmlElement.getAttributeInteger("Size");
+
+    // Size
+    setSize(uiSizeInBits / 8);
 
     // Min / Max
     if (_bSigned) {
 
         // Signed means we have one less util bit
-        uiUtilSizeInBits--;
+        uiSizeInBits--;
 
         if (xmlElement.hasAttribute("Min")) {
 
             _uiMin = (uint32_t)xmlElement.getAttributeSignedInteger("Min");
         } else {
 
-            _uiMin = 1UL << uiUtilSizeInBits;
+            _uiMin = 1UL << uiSizeInBits;
 
             signExtend((int32_t&)_uiMin);
         }
@@ -97,7 +97,7 @@ bool CIntegerParameterType::fromXml(const CXmlElement& xmlElement, CXmlSerializi
             _uiMax = (uint32_t)xmlElement.getAttributeSignedInteger("Max");
         } else {
 
-            _uiMax = (1UL << uiUtilSizeInBits) - 1;
+            _uiMax = (1UL << uiSizeInBits) - 1;
         }
     } else {
         if (xmlElement.hasAttribute("Min")) {
@@ -112,7 +112,7 @@ bool CIntegerParameterType::fromXml(const CXmlElement& xmlElement, CXmlSerializi
             _uiMax = xmlElement.getAttributeInteger("Max");
         } else {
 
-            _uiMax = (uint32_t)((1UL << uiUtilSizeInBits) - 1);
+            _uiMax = -1L >> (32 - uiSizeInBits);
         }
     }
 
@@ -136,13 +136,10 @@ bool CIntegerParameterType::asInteger(const string& strValue, uint32_t& uiValue,
         iData = strtol(strValue.c_str(), NULL, 0);
     }
 
-    if (bValueProvidedAsHexa) {
+    if (bValueProvidedAsHexa && isEncodable(iData)) {
 
-        if (isEncodable(iData)) {
-
-            // Sign extend
-            signExtend(iData);
-        }
+        // Sign extend
+        signExtend(iData);
     }
     // Check against Min / Max
     if (_bSigned) {
diff --git a/parameter/ParameterType.cpp b/parameter/ParameterType.cpp
index 0d3dac5..b22cde4 100644
--- a/parameter/ParameterType.cpp
+++ b/parameter/ParameterType.cpp
@@ -113,8 +113,8 @@ CInstanceConfigurableElement* CParameterType::doInstantiate() const
 // Sign extension
 void CParameterType::signExtend(int32_t& iData) const
 {
-    uint32_t uiSizeInBits = _uiSize << 3;
-    uint32_t uiShift = 32 - uiSizeInBits;
+    uint32_t uiSizeInBits = _uiSize * 8;
+    uint32_t uiShift = 8 * sizeof(iData) - uiSizeInBits;
 
     if (uiShift) {
 
@@ -125,13 +125,9 @@ void CParameterType::signExtend(int32_t& iData) const
 // Check data has no bit set outside available range
 bool CParameterType::isEncodable(uint32_t uiData) const
 {
-    return isEncodable(uiData, _uiSize << 3);
-}
+    uint32_t uiSizeInBits = _uiSize * 8;
 
-// Check data has no bit set outside available range
-bool CParameterType::isEncodable(uint32_t uiData, uint32_t uiSizeInBits) const
-{
-    if (uiSizeInBits == 32) {
+    if (uiSizeInBits == 8 * sizeof(uiData)) {
 
         return true;
     }
@@ -147,7 +143,7 @@ uint32_t CParameterType::makeEncodable(uint32_t uiData) const
 
         return uiData;
     }
-    uint32_t uiSizeInBits = _uiSize << 3;
+    uint32_t uiSizeInBits = _uiSize * 8;
 
     uint32_t uiMask = (1 << uiSizeInBits) - 1;
 
diff --git a/parameter/ParameterType.h b/parameter/ParameterType.h
index 980afba..e8e67a5 100644
--- a/parameter/ParameterType.h
+++ b/parameter/ParameterType.h
@@ -72,8 +72,6 @@ protected:
     void signExtend(int32_t& iData) const;
     // Check data has no bit set outside available range (based on byte size)
     bool isEncodable(uint32_t uiData) const;
-    // Check data has no bit set outside available range
-    bool isEncodable(uint32_t uiData, uint32_t uiSizeInBits) const;
     // Remove all bits set outside available range
     uint32_t makeEncodable(uint32_t uiData) const;