Merge pull request #352 from dawagner/fix-showMapping-segfault

Fix a segfault in the showMapping command.

Author: dawagner

parameter/ComponentInstance.cpp

@@ -73,14 +73,7 @@ bool CComponentInstance::hasMappingData() const
 
 std::string CComponentInstance::getFormattedMapping() const
 {
-    // Try myself first then associated component type
-    std::string strValue = base::getFormattedMapping();
-    if (_pComponentType) {
-
-        strValue += _pComponentType->getFormattedMapping();
-    }
-
-    return strValue;
+    return base::getFormattedMapping(_pComponentType);
 }
 
 bool CComponentInstance::fromXml(const CXmlElement &xmlElement,

parameter/ComponentType.cpp

@@ -65,14 +65,7 @@ bool CComponentType::hasMappingData() const
 
 std::string CComponentType::getFormattedMapping() const
 {
-    // Try myself first then associated component type
-    std::string strValue = base::getFormattedMapping();
-    if (_pExtendsComponentType) {
-
-        strValue += _pExtendsComponentType->getFormattedMapping();
-    }
-
-    return strValue;
+    return base::getFormattedMapping(_pExtendsComponentType);
 }
 
 bool CComponentType::fromXml(const CXmlElement &xmlElement,

parameter/ConfigurableElement.h

@@ -154,6 +154,11 @@ class PARAMETER_EXPORT CConfigurableElement : public CElement
      * @return true if @p strKey is found in the object's mapping, false if not
      */
     virtual bool getMappingData(const std::string &strKey, const std::string *&pStrValue) const = 0;
+    /** Get the string representation of the mapping
+     *
+     * If applicable, amend values are applied to the leaf element.
+     */
+    virtual std::string getFormattedMapping() const = 0;
 
     // XML configuration settings parsing
     virtual bool serializeXmlSettings(

parameter/InstanceConfigurableElement.h

@@ -67,7 +67,7 @@ class PARAMETER_EXPORT CInstanceConfigurableElement : public CConfigurableElemen
      *
      * @return A std::string containing the formatted mapping
      */
-    std::string getFormattedMapping() const;
+    std::string getFormattedMapping() const override;
 
     // From CElement
     virtual std::string getKind() const;

parameter/Subsystem.cpp

@@ -42,7 +42,6 @@
 
 using std::string;
 using std::list;
-using std::ostringstream;
 
 CSubsystem::CSubsystem(const string &strName, core::log::Logger &logger)
     : base(strName), _pComponentLibrary(new CComponentLibrary),
@@ -186,38 +185,38 @@ string CSubsystem::formatMappingDataList(
 {
     // The list is parsed in reverse order because it has been filled from the leaf to the trunk
     // of the tree. When formatting the mapping, we want to start from the subsystem level
-    ostringstream ossStream;
+    std::list<string> mappings;
     list<const CConfigurableElement *>::const_reverse_iterator it;
     for (it = configurableElementPath.rbegin(); it != configurableElementPath.rend(); ++it) {
 
-        const CInstanceConfigurableElement *pInstanceConfigurableElement =
-            static_cast<const CInstanceConfigurableElement *>(*it);
-
-        ossStream << pInstanceConfigurableElement->getFormattedMapping() << ", ";
+        auto maybeMapping = (*it)->getFormattedMapping();
+        if (not maybeMapping.empty()) {
+            mappings.push_back(maybeMapping);
+        }
     }
-    return ossStream.str();
+
+    return utility::asString(mappings, ", ");
 }
 
 // Find the CSubystemObject containing a specific CInstanceConfigurableElement
 const CSubsystemObject *CSubsystem::findSubsystemObjectFromConfigurableElement(
     const CInstanceConfigurableElement *pInstanceConfigurableElement) const
 {
 
-    const CSubsystemObject *pSubsystemObject = NULL;
-
     list<CSubsystemObject *>::const_iterator it;
     for (it = _subsystemObjectList.begin(); it != _subsystemObjectList.end(); ++it) {
 
         // Check if one of the SubsystemObjects is associated with a ConfigurableElement
         // corresponding to the expected one
-        pSubsystemObject = *it;
+        const CSubsystemObject *pSubsystemObject = *it;
+
         if (pSubsystemObject->getConfigurableElement() == pInstanceConfigurableElement) {
 
-            break;
+            return pSubsystemObject;
         }
     }
 
-    return pSubsystemObject;
+    return nullptr;
 }
 
 void CSubsystem::findSubsystemLevelMappingKeyValue(
@@ -277,14 +276,16 @@ string CSubsystem::getMapping(list<const CConfigurableElement *> &configurableEl
     // Get the first element, which is the element containing the amended mapping
     const CInstanceConfigurableElement *pInstanceConfigurableElement =
         static_cast<const CInstanceConfigurableElement *>(configurableElementPath.front());
-    configurableElementPath.pop_front();
-    // Now the list only contains elements whose mapping are related to the context
 
     // Format context mapping data
     string strValue = formatMappingDataList(configurableElementPath);
 
     // Print the mapping of the first node, which corresponds to a SubsystemObject
-    strValue += getFormattedSubsystemMappingData(pInstanceConfigurableElement);
+    auto subsystemObjectAmendedMapping =
+        getFormattedSubsystemMappingData(pInstanceConfigurableElement);
+    if (not subsystemObjectAmendedMapping.empty()) {
+        strValue += ", " + subsystemObjectAmendedMapping;
+    }
 
     return strValue;
 }
@@ -330,6 +331,15 @@ bool CSubsystem::getMappingData(const std::string &strKey, const std::string *&p
     return false;
 }
 
+// Returns the formatted mapping
+std::string CSubsystem::getFormattedMapping() const
+{
+    if (!_pMappingData) {
+        return "";
+    }
+    return _pMappingData->asString();
+}
+
 // Mapping generic context handling
 bool CSubsystem::handleMappingContext(const CConfigurableElement *pConfigurableElement,
                                       CMappingContext &context, string &strError) const

parameter/Subsystem.h

@@ -76,6 +76,7 @@ class PARAMETER_EXPORT CSubsystem : public CConfigurableElement, private IMapper
     virtual std::string getKind() const;
 
     virtual bool getMappingData(const std::string &strKey, const std::string *&pStrValue) const;
+    std::string getFormattedMapping() const override;
 
     /**
      * Fetch mapping data of an element.

parameter/SystemClass.cpp

@@ -87,6 +87,11 @@ bool CSystemClass::getMappingData(const std::string & /*strKey*/,
     return false;
 }
 
+string CSystemClass::getFormattedMapping() const
+{
+    return "";
+}
+
 bool CSystemClass::loadSubsystems(string &strError, const CSubsystemPlugins *pSubsystemPlugins,
                                   bool bVirtualSubsystemFallback)
 {

parameter/SystemClass.h

@@ -84,6 +84,7 @@ class CSystemClass final : public CConfigurableElement
     virtual std::string getKind() const;
 
     bool getMappingData(const std::string &strKey, const std::string *&pStrValue) const override;
+    std::string getFormattedMapping() const override;
 
 private:
     CSystemClass(const CSystemClass &);

parameter/TypeElement.cpp

@@ -31,6 +31,8 @@
 #include "MappingData.h"
 #include "Tokenizer.h"
 #include "InstanceConfigurableElement.h"
+#include "Utility.h"
+#include <list>
 #include <assert.h>
 
 #define base CElement
@@ -141,6 +143,30 @@ CMappingData *CTypeElement::getMappingData()
     return _pMappingData;
 }
 
+std::string CTypeElement::getFormattedMapping(const CTypeElement *predecessor) const
+{
+    std::list<std::string> mappings;
+    std::string mapping;
+
+    // Try predecessor type first, then myself (in order to have higher-level
+    // mappings displayed first)
+    if (predecessor) {
+        mapping = predecessor->getFormattedMapping();
+        if (not mapping.empty()) {
+            mappings.push_back(mapping);
+        }
+    }
+
+    // Explicitly call the root implementation instead of calling it virtually
+    // (otherwise, it will infinitely recurse).
+    mapping = CTypeElement::getFormattedMapping();
+    if (not mapping.empty()) {
+        mappings.push_back(mapping);
+    }
+
+    return utility::asString(mappings, ", ");
+}
+
 std::string CTypeElement::getFormattedMapping() const
 {
     if (_pMappingData) {

parameter/TypeElement.h

@@ -84,6 +84,17 @@ class PARAMETER_EXPORT CTypeElement : public CElement
 protected:
     // Object creation
     virtual void populate(CElement *pElement) const;
+    /** @Returns the mapping associated to the current type and its predecessor
+     *
+     * The meaning of predecessor depends on the TypeElement type: e.g. for a
+     * component instance, the predecessor is the ComponentType; for a
+     * ComponentType, the predecessor is its base type.
+     *
+     * The predecessor's mapping comes first, then the current type's mapping.
+     *
+     * @param[in] predecessor A pointer to the predecessor or NULL.
+     */
+    std::string getFormattedMapping(const CTypeElement *predecessor) const;
 
 private:
     CTypeElement(const CTypeElement &);