diff --git a/parameter/ConfigurableDomain.cpp b/parameter/ConfigurableDomain.cpp index eb3ef834e..2a743072b 100644 --- a/parameter/ConfigurableDomain.cpp +++ b/parameter/ConfigurableDomain.cpp @@ -686,8 +686,6 @@ bool CConfigurableDomain::deleteConfiguration(const string &strName, string &str void CConfigurableDomain::listAssociatedToElements(string &strResult) const { - strResult = "\n"; - ConfigurableElementListIterator it; // Browse all configurable elements diff --git a/parameter/ConfigurableDomains.cpp b/parameter/ConfigurableDomains.cpp index 734a76263..7c5108381 100644 --- a/parameter/ConfigurableDomains.cpp +++ b/parameter/ConfigurableDomains.cpp @@ -317,8 +317,6 @@ bool CConfigurableDomains::split(const string &domainName, CConfigurableElement void CConfigurableDomains::listAssociatedElements(string &strResult) const { - strResult = "\n"; - std::set configurableElementSet; // Get all owned configurable elements @@ -341,8 +339,6 @@ void CConfigurableDomains::listAssociatedElements(string &strResult) const void CConfigurableDomains::listConflictingElements(string &strResult) const { - strResult = "\n"; - std::set configurableElementSet; // Get all owned configurable elements @@ -369,8 +365,6 @@ void CConfigurableDomains::listConflictingElements(string &strResult) const void CConfigurableDomains::listDomains(string &strResult) const { - strResult = "\n"; - // List domains size_t uiNbConfigurableDomains = getNbChildren(); diff --git a/parameter/ConfigurableElement.cpp b/parameter/ConfigurableElement.cpp index 991523731..615d72b44 100644 --- a/parameter/ConfigurableElement.cpp +++ b/parameter/ConfigurableElement.cpp @@ -468,8 +468,6 @@ void CConfigurableElement::listBelongingDomains(std::string &strResult, bool bVe // Elements with no domains void CConfigurableElement::listRogueElements(std::string &strResult) const { - strResult = "\n"; - // Get rogue element aggregate list (no associated domain) std::list rogueElementList; @@ -568,11 +566,6 @@ void CConfigurableElement::listDomains( const std::list &configurableDomainList, std::string &strResult, bool bVertical) const { - if (bVertical && configurableDomainList.empty()) { - - strResult = "\n"; - } - // Fill list ConfigurableDomainListConstIterator it; bool bFirst = true; diff --git a/parameter/Element.cpp b/parameter/Element.cpp index db80a371b..77dfd9d33 100644 --- a/parameter/Element.cpp +++ b/parameter/Element.cpp @@ -122,7 +122,6 @@ string CElement::dumpContent(utility::ErrorContext &errorContext, const size_t d // Element properties void CElement::showProperties(string &strResult) const { - strResult = "\n"; strResult += "Kind: " + getKind() + "\n"; showDescriptionProperty(strResult); } @@ -330,8 +329,6 @@ bool CElement::removeChild(CElement *pChild) void CElement::listChildren(string &strChildList) const { - strChildList = "\n"; - // Get list of children names for (CElement *pChild : _childArray) { diff --git a/parameter/MappingData.cpp b/parameter/MappingData.cpp index 9f28152ec..cc341038b 100644 --- a/parameter/MappingData.cpp +++ b/parameter/MappingData.cpp @@ -38,7 +38,7 @@ bool CMappingData::init(const std::string &rawMapping, std::string &error) std::string strMappingElement; - while (!(strMappingElement = mappingTok.next()).empty()) { + for (const auto &strMappingElement : mappingTok.split()) { std::string::size_type iFistDelimiterOccurrence = strMappingElement.find_first_of(':'); diff --git a/parameter/ParameterMgr.cpp b/parameter/ParameterMgr.cpp index 3d6fb878e..8ffd7f09d 100644 --- a/parameter/ParameterMgr.cpp +++ b/parameter/ParameterMgr.cpp @@ -1370,8 +1370,6 @@ CParameterMgr::CCommandHandler::CommandStatus CParameterMgr::listElementsCommand return CCommandHandler::EFailed; } - strResult = string("\n"); - if (!pLocatedElement) { // List from root folder @@ -1399,8 +1397,6 @@ CParameterMgr::CCommandHandler::CommandStatus CParameterMgr::listParametersComma return CCommandHandler::EFailed; } - strResult = string("\n"); - if (!pLocatedElement) { // List from root folder diff --git a/remote-processor/RemoteCommandHandlerTemplate.h b/remote-processor/RemoteCommandHandlerTemplate.h index 66269fbdb..db59a5b3f 100644 --- a/remote-processor/RemoteCommandHandlerTemplate.h +++ b/remote-processor/RemoteCommandHandlerTemplate.h @@ -194,8 +194,6 @@ class TRemoteCommandHandlerTemplate : public IRemoteCommandHandler { initMaxCommandUsageLength(); - strResult = "\n"; - // Show usages for (const auto *pRemoteCommandParserItem : _remoteCommandParserVector) { diff --git a/test/tokenizer/Test.cpp b/test/tokenizer/Test.cpp index d9c48ffae..37a72cbff 100644 --- a/test/tokenizer/Test.cpp +++ b/test/tokenizer/Test.cpp @@ -39,76 +39,74 @@ using std::string; using std::vector; +using Expected = vector; + SCENARIO("Tokenizer tests") { GIVEN ("A default tokenizer") { GIVEN ("A trivial string") { Tokenizer tokenizer("a bcd ef"); + Expected expected{"a", "bcd", "ef"}; - THEN ("next() api should work") { - CHECK(tokenizer.next() == "a"); - CHECK(tokenizer.next() == "bcd"); - CHECK(tokenizer.next() == "ef"); - CHECK(tokenizer.next() == ""); - } THEN ("split() api should work") { - vector expected; - expected.push_back("a"); - expected.push_back("bcd"); - expected.push_back("ef"); - CHECK(tokenizer.split() == expected); } } GIVEN ("An empty string") { Tokenizer tokenizer(""); + Expected expected{}; - THEN ("next() api should work") { - CHECK(tokenizer.next() == ""); + THEN ("split() should be empty") { + CHECK(tokenizer.split() == expected); } - THEN ("split() api should work") { - vector expected; + } - CHECK(tokenizer.split().empty()); + GIVEN ("Multiple separators in a row") { + Tokenizer tokenizer(" a \n\t bc "); + Expected expected{"a", "bc"}; + + THEN ("split() api should work") { + CHECK(tokenizer.split() == expected); } } + } - GIVEN ("A slash-separated string and tokenizer") { - Tokenizer tokenizer("/a/bcd/ef g/h/", "/"); + GIVEN ("A slash-separated string and tokenizer") { + Tokenizer tokenizer("/a/bcd/ef g/h/", "/"); + Expected expected{"a", "bcd", "ef g", "h"}; - THEN ("next() api should work") { - CHECK(tokenizer.next() == "a"); - CHECK(tokenizer.next() == "bcd"); - CHECK(tokenizer.next() == "ef g"); - CHECK(tokenizer.next() == "h"); - CHECK(tokenizer.next() == ""); + THEN ("split() api should work") { + CHECK(tokenizer.split() == expected); + } + } + + GIVEN ("A tokenizer that doesn't merge consecutive separators") { + + GIVEN ("An empty string") { + Tokenizer tokenizer("", Tokenizer::defaultDelimiters, false); + Expected expected{}; + + THEN ("split() should be empty") { + CHECK(tokenizer.split() == expected); } - THEN ("split() api should work") { - vector expected; - expected.push_back("a"); - expected.push_back("bcd"); - expected.push_back("ef g"); - expected.push_back("h"); + } + + GIVEN ("A string consisting only of a delimiter") { + Tokenizer tokenizer(",", ",", false); + Expected expected{"", ""}; + THEN ("split() should return two empty tokens") { CHECK(tokenizer.split() == expected); } } GIVEN ("Multiple separators in a row") { - Tokenizer tokenizer(" a \n\t bc "); + Tokenizer tokenizer(" a b \nc d ", Tokenizer::defaultDelimiters, false); + Expected expected{"", "a", "", "b", "", "c", "d", ""}; - THEN ("next() api should work") { - CHECK(tokenizer.next() == "a"); - CHECK(tokenizer.next() == "bc"); - CHECK(tokenizer.next() == ""); - } THEN ("split() api should work") { - vector expected; - expected.push_back("a"); - expected.push_back("bc"); - CHECK(tokenizer.split() == expected); } } diff --git a/test/xml-generator/PFConfig/duplicate_criterion_value.txt b/test/xml-generator/PFConfig/duplicate_criterion_value.txt new file mode 100644 index 000000000..af5ffe6de --- /dev/null +++ b/test/xml-generator/PFConfig/duplicate_criterion_value.txt @@ -0,0 +1 @@ +InclusiveCriterion Duplicate : Foo Foo diff --git a/test/xml-generator/PFConfig/structure.xml b/test/xml-generator/PFConfig/structure.xml index 828e2047f..e9b621fc6 100644 --- a/test/xml-generator/PFConfig/structure.xml +++ b/test/xml-generator/PFConfig/structure.xml @@ -10,6 +10,9 @@ + + + diff --git a/test/xml-generator/test.py b/test/xml-generator/test.py index 4345bd4c7..80287f3f7 100644 --- a/test/xml-generator/test.py +++ b/test/xml-generator/test.py @@ -90,27 +90,42 @@ def check(self, reference=None, expectedErrors=0): config_dir = os.path.join(basedir, "PFConfig") vector_dir = os.path.join(basedir, "testVector") class TestCase(unittest.TestCase): - nominal_reference = open(os.path.join(vector_dir, "reference.xml")).read().splitlines() - nominal_pfconfig = PfConfig(os.path.join(config_dir, "configuration.xml"), - os.path.join(config_dir, "criteria.txt"), - os.path.join(basedir, "../../schemas")) - nominal_vector = TestVector(os.path.join(vector_dir, "initialSettings.xml"), - [os.path.join(vector_dir, "first.pfw"), - os.path.join(vector_dir, "second.pfw"), - os.path.join(vector_dir, "complex.pfw")], - [os.path.join(vector_dir, "third.xml"), - os.path.join(vector_dir, "fourth.xml")]) + def setUp(self): + self.nominal_reference = open(os.path.join(vector_dir, "reference.xml")).read().splitlines() + self.nominal_pfconfig = PfConfig(os.path.join(config_dir, "configuration.xml"), + os.path.join(config_dir, "criteria.txt"), + os.path.join(basedir, "../../schemas")) + self.nominal_vector = TestVector(os.path.join(vector_dir, "initialSettings.xml"), + [os.path.join(vector_dir, "first.pfw"), + os.path.join(vector_dir, "second.pfw"), + os.path.join(vector_dir, "complex.pfw")], + [os.path.join(vector_dir, "third.xml"), + os.path.join(vector_dir, "fourth.xml")]) def test_nominal(self): tester = Tester(self.nominal_pfconfig, self.nominal_vector) tester.check(self.nominal_reference) def test_nonfatalError(self): - vector = copy.copy(self.nominal_vector) - vector.edds.append(os.path.join(vector_dir, "duplicate.pfw")) + self.nominal_vector.edds.append(os.path.join(vector_dir, "duplicate.pfw")) - tester = Tester(self.nominal_pfconfig, vector) + tester = Tester(self.nominal_pfconfig, self.nominal_vector) tester.check(self.nominal_reference, expectedErrors=1) + def test_conflicting(self): + vector = TestVector(edds=[os.path.join(vector_dir, "conflicting.pfw")]) + + tester = Tester(self.nominal_pfconfig, vector) + tester.check(expectedErrors=1) + + def test_fail_criteria(self): + self.nominal_pfconfig.criteria = os.path.join(config_dir, "duplicate_criterion_value.txt") + # Empty test vector: we want to make sure that the erroneous criterion + # is the only source of error + empty_vector = TestVector() + + tester = Tester(self.nominal_pfconfig, empty_vector) + tester.check(expectedErrors=1) + if __name__ == "__main__": unittest.main() diff --git a/test/xml-generator/testVector/complex.pfw b/test/xml-generator/testVector/complex.pfw index 042259572..6339a1310 100644 --- a/test/xml-generator/testVector/complex.pfw +++ b/test/xml-generator/testVector/complex.pfw @@ -33,8 +33,8 @@ domainGroup: Red sequenceAware Colors Includes Blue component: /Test/test - component: block/3 + component: block/0 q2.5 = 1.18750 string = 12 ab @ \n \0 ✔ "'\ - component: included + component: inline bool = 1 diff --git a/test/xml-generator/testVector/conflicting.pfw b/test/xml-generator/testVector/conflicting.pfw new file mode 100644 index 000000000..55544615d --- /dev/null +++ b/test/xml-generator/testVector/conflicting.pfw @@ -0,0 +1,7 @@ +# Causes a conflicting-element error +domain: First + conf: Default + /Test/test/included/bool = 1 +domain: Second + conf: Default + /Test/test/included/bool = 0 diff --git a/test/xml-generator/testVector/first.pfw b/test/xml-generator/testVector/first.pfw index b049459fb..a8cd66428 100644 --- a/test/xml-generator/testVector/first.pfw +++ b/test/xml-generator/testVector/first.pfw @@ -15,7 +15,8 @@ domainGroup: EddGroup component: /Test/test/block/1 q2.5 = 1.2 - string = some string + # set to an empty string + string = # Inherits from "EddGroup" domainGroup, "First" domain # and from "Green" confGroup diff --git a/test/xml-generator/testVector/reference.xml b/test/xml-generator/testVector/reference.xml index 33c1e95ba..a07354522 100644 --- a/test/xml-generator/testVector/reference.xml +++ b/test/xml-generator/testVector/reference.xml @@ -94,7 +94,7 @@ 1.18750 - some string + @@ -200,19 +200,19 @@ - - - + + + - + 1.18750 - + 12 ab @ <![CDATA[ < > \n \0 ✔ "'\ - + 1 diff --git a/tools/xmlGenerator/domainGeneratorConnector.cpp b/tools/xmlGenerator/domainGeneratorConnector.cpp index b0fdbfef6..a83822f8d 100644 --- a/tools/xmlGenerator/domainGeneratorConnector.cpp +++ b/tools/xmlGenerator/domainGeneratorConnector.cpp @@ -92,6 +92,14 @@ class XmlGenerator */ size_t parse(std::istream &input); + /** Check for elements belonging to several domains + * + * Prints conflicting elements, if any, on the error output. + * + * @returns true if there are conflicting elements, false otherwise + */ + bool conflictingElements(); + /** Prints the Parameter Framework's instance configuration * * @param[out] output The stream to which output the configuration @@ -155,7 +163,7 @@ size_t XmlGenerator::parse(std::istream &input) while (not input.eof()) { std::getline(std::cin, line); - auto tokens = Tokenizer(line, string(1, '\0')).split(); + auto tokens = Tokenizer(line, string(1, '\0'), false).split(); if (tokens.empty()) { continue; } @@ -182,6 +190,22 @@ size_t XmlGenerator::parse(std::istream &input) return errorNb; } +bool XmlGenerator::conflictingElements() +{ + string conflicting; + if (not mCommandHandler->process("listConflictingElements", {}, conflicting)) { + // Should not happen + throw Exception("Failed to list conflicting elements"); + } + + if (not conflicting.empty()) { + std::cerr << "There are conflicting elements:" << std::endl << conflicting; + return true; + } + + return false; +} + void XmlGenerator::start() { string error; @@ -257,7 +281,9 @@ int main(int argc, char *argv[]) try { XmlGenerator xmlGenerator(toplevelConfig, validate, verbose, schemasDir); auto errorNb = xmlGenerator.parse(std::cin); - // TODO: add a check for conflicting elements + if (xmlGenerator.conflictingElements()) { + errorNb++; + } xmlGenerator.exportDomains(std::cout); return normalizeExitCode(errorNb); diff --git a/utility/Tokenizer.cpp b/utility/Tokenizer.cpp index a4cfcf03b..13f4319a1 100644 --- a/utility/Tokenizer.cpp +++ b/utility/Tokenizer.cpp @@ -34,42 +34,45 @@ using std::vector; const string Tokenizer::defaultDelimiters = " \n\r\t\v\f"; -Tokenizer::Tokenizer(const string &input, const string &delimiters) - : _input(input), _delimiters(delimiters), _position(0) +Tokenizer::Tokenizer(const string &input, const string &delimiters, bool mergeDelimiters) + : _input(input), _delimiters(delimiters), _mergeDelimiters(mergeDelimiters) { } -string Tokenizer::next() -{ - string token; - - // Skip all leading delimiters - string::size_type tokenStart = _input.find_first_not_of(_delimiters, _position); - - // Special case if there isn't any token anymore (string::substr's - // throws when pos==npos) - if (tokenStart == string::npos) { - return ""; - } - - // Starting from the token's start, find the first delimiter - string::size_type tokenEnd = _input.find_first_of(_delimiters, tokenStart); - - _position = tokenEnd; - - return _input.substr(tokenStart, tokenEnd - tokenStart); -} - vector Tokenizer::split() { vector result; string token; + bool leftover = false; + + for (const auto character : _input) { + if (_delimiters.find(character) != string::npos) { + if (_mergeDelimiters) { + leftover = false; + if (token.empty()) { + // skip consecutive delimiters + continue; + } + } else { + // We've encountered a delimiter, which means that there is a + // left-hand token and a right-side token. We are going to add + // the left-hand one but must not forget that there is a + // right-hand one (possibly empty) + leftover = true; + } - while (true) { - token = next(); - if (token.empty()) { - return result; + result.push_back(token); + token.clear(); + continue; } + token += character; + leftover = true; + } + + // push any leftover token: + if (leftover) { result.push_back(token); } + + return result; } diff --git a/utility/Tokenizer.h b/utility/Tokenizer.h index 0e38c6b51..1974041f5 100644 --- a/utility/Tokenizer.h +++ b/utility/Tokenizer.h @@ -38,9 +38,6 @@ * * Must be initialized with a string to be tokenized and, optionally, a string * of delimiters (@see Tokenizer::defaultDelimiters). - * - * Multiple consecutive delimiters (even if different) are considered as a - * single one. As a result, there can't be empty tokens. */ class Tokenizer : private utility::NonCopyable { @@ -50,18 +47,14 @@ class Tokenizer : private utility::NonCopyable * @param[in] input The string to be tokenized * @param[in] delimiters A string containing all the token delimiters * (hence, each delimiter can only be a single character) + * @param[in] mergeDelimiters If true, consecutive delimiters are considered + * as one; leading and trailing delimiters are also ignored. + * If false, consecutive delimiters produce empty tokens */ - Tokenizer(const std::string &input, const std::string &delimiters = defaultDelimiters); + Tokenizer(const std::string &input, const std::string &delimiters = defaultDelimiters, + bool mergeDelimiters = true); ~Tokenizer(){}; - /** Return the next token or an empty string if no more token - * - * Multiple consecutive delimiters are considered as a single one - i.e. - * "a bc d " will be tokenized as ("a", "bc", "d") if the delimiter - * is ' '. - */ - std::string next(); - /** Return a vector of all tokens */ std::vector split(); @@ -72,6 +65,5 @@ class Tokenizer : private utility::NonCopyable private: const std::string _input; //< string to be tokenized const std::string _delimiters; //< token delimiters - - std::string::size_type _position; //< end of the last returned token + const bool _mergeDelimiters; //< whether subsequent delimiters should be merged };