Skip to content
This repository has been archived by the owner on Aug 5, 2022. It is now read-only.

RFC: modify the criterion APIs to take uints instead of ints #153

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

RFC: modify the criterion APIs to take uints instead of ints #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfe3d64
RFC: modify the criterion APIs to take uints instead of ints
dawagner Jul 22, 2015
c7c104d
test-platform: limit the number of inclusive criterion values to 32
dawagner Jul 22, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions bindings/c/ParameterFramework.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include <cassert>
#include <cstring>
#include <cstdlib>
#include <climits>

using std::string;

Expand Down Expand Up @@ -184,14 +185,14 @@ bool PfwHandler::createCriteria(const PfwCriterion criteriaArray[], size_t crite
assert(type != NULL);
// Add criterion values
for (size_t valueIndex = 0; criterion.values[valueIndex] != NULL; ++valueIndex) {
int value;
unsigned int value;
if (criterion.inclusive) {
// Check that (int)1 << valueIndex would not overflow (UB)
if(std::numeric_limits<int>::max() >> valueIndex == 0) {
// Check that we are not going to shift too far (UB)
if (valueIndex >= sizeof(unsigned int) * CHAR_BIT) {
return status.failure("Too many values for criterion " +
string(criterion.name));
}
value = 1 << valueIndex;
value = 1U << valueIndex;
} else {
value = valueIndex;
}
Expand Down
10 changes: 5 additions & 5 deletions bindings/c/Test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ TEST_CASE_METHOD(Test, "Parameter-framework c api use") {
}
GIVEN("A criteria with lots of values")
{
// Build a criterion with as many value as there is bits in int.
std::vector<char> names(sizeof(int) * CHAR_BIT + 1, 'a');
// Build a criterion with as many value as there is bits in unsigned int + 1
std::vector<char> names(sizeof(unsigned int) * CHAR_BIT + 2, 'a');
names.back() = '\0';
std::vector<const char *> values(names.size());
for(size_t i = 0; i < values.size(); ++i) {
Expand Down Expand Up @@ -238,14 +238,14 @@ TEST_CASE_METHOD(Test, "Parameter-framework c api use") {
* values[n] = NULL
*
*/
const PfwCriterion duplicatedCriteria[] = {{"name", true, &values[0]}};
const PfwCriterion longCriteria[] = {{"name", true, &values[0]}};

WHEN("The pfw is started with a too long criterion state list") {
REQUIRE_FAILURE(pfwStart(pfw, config, duplicatedCriteria, 1, &logger));
REQUIRE_FAILURE(pfwStart(pfw, config, longCriteria, 1, &logger));
}
WHEN("The pfw is started with max length criterion state list") {
values[values.size() - 2] = NULL; // Hide last value
REQUIRE_SUCCESS(pfwStart(pfw, config, duplicatedCriteria, 1, &logger));
REQUIRE_SUCCESS(pfwStart(pfw, config, longCriteria, 1, &logger));
}
}

Expand Down
8 changes: 4 additions & 4 deletions bindings/python/pfw.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,11 @@ class ISelectionCriterionTypeInterface
%}

public:
virtual bool addValuePair(int iValue, const std::string& strValue) = 0;
virtual bool getNumericalValue(const std::string& strValue, int& iValue) const = 0;
virtual bool getLiteralValue(int iValue, std::string& strValue) const = 0;
virtual bool addValuePair(unsigned int uiValue, const std::string& strValue) = 0;
virtual bool getNumericalValue(const std::string& strValue, unsigned int& uiValue) const = 0;
virtual bool getLiteralValue(unsigned int uiValue, std::string& strValue) const = 0;
virtual bool isTypeInclusive() const = 0;
virtual std::string getFormattedState(int iValue) const = 0;
virtual std::string getFormattedState(unsigned int uiValue) const = 0;

protected:
virtual ~ISelectionCriterionTypeInterface() {}
Expand Down
18 changes: 9 additions & 9 deletions parameter/SelectionCriterionRule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const CSelectionCriterionRule::SMatchingRuleDescription CSelectionCriterionRule:
{ "Excludes", false }
};

CSelectionCriterionRule::CSelectionCriterionRule() : _pSelectionCriterion(NULL), _eMatchesWhen(CSelectionCriterionRule::EIs), _iMatchValue(0)
CSelectionCriterionRule::CSelectionCriterionRule() : _pSelectionCriterion(NULL), _eMatchesWhen(CSelectionCriterionRule::EIs), _uiMatchValue(0)
{
}

Expand Down Expand Up @@ -104,7 +104,7 @@ bool CSelectionCriterionRule::parse(CRuleParser& ruleParser, string& strError)
}

// Value
if (!_pSelectionCriterion->getCriterionType()->getNumericalValue(strValue, _iMatchValue)) {
if (!_pSelectionCriterion->getCriterionType()->getNumericalValue(strValue, _uiMatchValue)) {

strError = "Value error: \"" + strValue + "\" is not part of criterion \"" +
_pSelectionCriterion->getCriterionName() + "\"";
Expand All @@ -126,7 +126,7 @@ void CSelectionCriterionRule::dump(string& strResult) const
strResult += " ";
// Value
string strValue;
_pSelectionCriterion->getCriterionType()->getLiteralValue(_iMatchValue, strValue);
_pSelectionCriterion->getCriterionType()->getLiteralValue(_uiMatchValue, strValue);
strResult += strValue;
}

Expand All @@ -137,13 +137,13 @@ bool CSelectionCriterionRule::matches() const

switch(_eMatchesWhen) {
case EIs:
return _pSelectionCriterion->is(_iMatchValue);
return _pSelectionCriterion->is(_uiMatchValue);
case EIsNot:
return _pSelectionCriterion->isNot(_iMatchValue);
return _pSelectionCriterion->isNot(_uiMatchValue);
case EIncludes:
return _pSelectionCriterion->includes(_iMatchValue);
return _pSelectionCriterion->includes(_uiMatchValue);
case EExcludes:
return _pSelectionCriterion->excludes(_iMatchValue);
return _pSelectionCriterion->excludes(_uiMatchValue);
default:
assert(0);
return false;
Expand Down Expand Up @@ -183,7 +183,7 @@ bool CSelectionCriterionRule::fromXml(const CXmlElement& xmlElement, CXmlSeriali
// Get Value
string strValue = xmlElement.getAttributeString("Value");

if (!_pSelectionCriterion->getCriterionType()->getNumericalValue(strValue, _iMatchValue)) {
if (!_pSelectionCriterion->getCriterionType()->getNumericalValue(strValue, _uiMatchValue)) {

xmlDomainImportContext.setError("Wrong Value attribute value " + strValue + " in " + getKind() + " " + xmlElement.getPath());

Expand All @@ -210,7 +210,7 @@ void CSelectionCriterionRule::toXml(CXmlElement& xmlElement, CXmlSerializingCont
// Set Value
string strValue;

_pSelectionCriterion->getCriterionType()->getLiteralValue(_iMatchValue, strValue);
_pSelectionCriterion->getCriterionType()->getLiteralValue(_uiMatchValue, strValue);

xmlElement.setAttributeString("Value", strValue);
}
Expand Down
2 changes: 1 addition & 1 deletion parameter/SelectionCriterionRule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class CSelectionCriterionRule : public CRule
MatchesWhen _eMatchesWhen;

// Value
int32_t _iMatchValue;
uint32_t _uiMatchValue;

// Used for XML MatchesWhen attribute parsing
static const SMatchingRuleDescription _astMatchesWhen[ENbMatchesWhen];
Expand Down
50 changes: 25 additions & 25 deletions parameter/SelectionCriterionType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,81 +49,81 @@ std::string CSelectionCriterionType::getKind() const
}

// From ISelectionCriterionTypeInterface
bool CSelectionCriterionType::addValuePair(int iValue, const std::string& strValue)
bool CSelectionCriterionType::addValuePair(unsigned int uiValue, const std::string& strValue)
{
// Check 1 bit set only for inclusive types
if (_bInclusive && (!iValue || (iValue & (iValue - 1)))) {
if (_bInclusive && (!uiValue || (uiValue & (uiValue - 1)))) {

log_warning("Rejecting value pair association: 0x%X - %s for Selection Criterion Type %s", iValue, strValue.c_str(), getName().c_str());
log_warning("Rejecting value pair association: 0x%X - %s for Selection Criterion Type %s", uiValue, strValue.c_str(), getName().c_str());

return false;
}

// Check already inserted
if (_numToLitMap.find(strValue) != _numToLitMap.end()) {

log_warning("Rejecting value pair association (literal already present): 0x%X - %s for Selection Criterion Type %s", iValue, strValue.c_str(), getName().c_str());
log_warning("Rejecting value pair association (literal already present): 0x%X - %s for Selection Criterion Type %s", uiValue, strValue.c_str(), getName().c_str());

return false;
}
for (NumToLitMapConstIt it = _numToLitMap.begin(); it != _numToLitMap.end(); ++it) {
if (it->second == iValue) {
if (it->second == uiValue) {
log_warning("Rejecting value pair association (numerical already present): 0x%X - %s"
" for Selection Criterion Type %s",
iValue, strValue.c_str(), getName().c_str());
uiValue, strValue.c_str(), getName().c_str());
return false;
}
}
_numToLitMap[strValue] = iValue;
_numToLitMap[strValue] = uiValue;

return true;
}

bool CSelectionCriterionType::getNumericalValue(const std::string& strValue, int& iValue) const
bool CSelectionCriterionType::getNumericalValue(const std::string& strValue, unsigned int& uiValue) const
{
if (_bInclusive) {

Tokenizer tok(strValue, _strDelimiter);
std::vector<std::string> astrValues = tok.split();
size_t uiNbValues = astrValues.size();
int iResult = 0;
size_t uiValueIndex;
iValue = 0;
unsigned int uiResult = 0;
size_t uuiValueIndex;
uiValue = 0;

// Looping on each std::string delimited by "|" token and adding the associated value
for (uiValueIndex = 0; uiValueIndex < uiNbValues; uiValueIndex++) {
for (uuiValueIndex = 0; uuiValueIndex < uiNbValues; uuiValueIndex++) {

if (!getAtomicNumericalValue(astrValues[uiValueIndex], iResult)) {
if (!getAtomicNumericalValue(astrValues[uuiValueIndex], uiResult)) {

return false;
}
iValue |= iResult;
uiValue |= uiResult;
}
return true;
}
return getAtomicNumericalValue(strValue, iValue);
return getAtomicNumericalValue(strValue, uiValue);
}

bool CSelectionCriterionType::getAtomicNumericalValue(const std::string& strValue, int& iValue) const
bool CSelectionCriterionType::getAtomicNumericalValue(const std::string& strValue, unsigned int& uiValue) const
{
NumToLitMapConstIt it = _numToLitMap.find(strValue);

if (it != _numToLitMap.end()) {

iValue = it->second;
uiValue = it->second;

return true;
}
return false;
}

bool CSelectionCriterionType::getLiteralValue(int iValue, std::string& strValue) const
bool CSelectionCriterionType::getLiteralValue(unsigned int uiValue, std::string& strValue) const
{
NumToLitMapConstIt it;

for (it = _numToLitMap.begin(); it != _numToLitMap.end(); ++it) {

if (it->second == iValue) {
if (it->second == uiValue) {

strValue = it->first;

Expand Down Expand Up @@ -164,7 +164,7 @@ std::string CSelectionCriterionType::listPossibleValues() const
}

// Formatted state
std::string CSelectionCriterionType::getFormattedState(int iValue) const
std::string CSelectionCriterionType::getFormattedState(unsigned int uiValue) const
{
std::string strFormattedState;

Expand All @@ -174,20 +174,20 @@ std::string CSelectionCriterionType::getFormattedState(int iValue) const
uint32_t uiBit;
bool bFirst = true;

for (uiBit = 0; uiBit < sizeof(iValue) * 8; uiBit++) {
for (uiBit = 0; uiBit < sizeof(uiValue) * 8; uiBit++) {

int iSingleBitValue = iValue & (1 << uiBit);
unsigned int uiSingleBitValue = uiValue & (1 << uiBit);

// Check if current bit is set
if (!iSingleBitValue) {
if (!uiSingleBitValue) {

continue;
}

// Simple translation
std::string strSingleValue;

if (!getLiteralValue(iSingleBitValue, strSingleValue)) {
if (!getLiteralValue(uiSingleBitValue, strSingleValue)) {
// Numeric value not part supported values for this criterion type.
continue;
}
Expand All @@ -204,7 +204,7 @@ std::string CSelectionCriterionType::getFormattedState(int iValue) const

} else {
// Simple translation
getLiteralValue(iValue, strFormattedState);
getLiteralValue(uiValue, strFormattedState);
}

// Sometimes nothing is set
Expand Down
16 changes: 8 additions & 8 deletions parameter/SelectionCriterionType.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,32 +36,32 @@

class CSelectionCriterionType : public CElement, public ISelectionCriterionTypeInterface
{
typedef std::map<std::string, int>::const_iterator NumToLitMapConstIt;
typedef std::map<std::string, unsigned int>::const_iterator NumToLitMapConstIt;

public:
CSelectionCriterionType(bool bIsInclusive);

// From ISelectionCriterionTypeInterface
virtual bool addValuePair(int iValue, const std::string& strValue);
virtual bool addValuePair(unsigned int uiValue, const std::string& strValue);
/**
* Retrieve the numerical value from the std::string representation of the criterion type.
*
* @param[in] strValue: criterion type value represented as a stream. If the criterion is
* inclusive, it supports more than one criterion type value delimited
* by the "|" symbol.
* @param[out] iValue: criterion type value represented as an integer.
* @param[out] uiValue: criterion type value represented as an unsigned integer.
*
* @return true if integer value retrieved from the std::string one, false otherwise.
*/
virtual bool getNumericalValue(const std::string& strValue, int& iValue) const;
virtual bool getLiteralValue(int iValue, std::string& strValue) const;
virtual bool getNumericalValue(const std::string& strValue, unsigned int& uiValue) const;
virtual bool getLiteralValue(unsigned int uiValue, std::string& strValue) const;
virtual bool isTypeInclusive() const;

// Value list
std::string listPossibleValues() const;

// Formatted state
virtual std::string getFormattedState(int iValue) const;
virtual std::string getFormattedState(unsigned int uiValue) const;

/**
* Export to XML
Expand All @@ -84,9 +84,9 @@ class CSelectionCriterionType : public CElement, public ISelectionCriterionTypeI
*
* @return true if integer value retrieved from the std::string one, false otherwise.
*/
bool getAtomicNumericalValue(const std::string& strValue, int& iValue) const;
bool getAtomicNumericalValue(const std::string& strValue, unsigned int& uiValue) const;
bool _bInclusive;
std::map<std::string, int> _numToLitMap;
std::map<std::string, unsigned int> _numToLitMap;

static const std::string _strDelimiter; /**< Inclusive criterion type delimiter. */
};
Expand Down
14 changes: 10 additions & 4 deletions parameter/include/SelectionCriterionTypeInterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,17 @@
class ISelectionCriterionTypeInterface
{
public:
virtual bool addValuePair(int iValue, const std::string& strValue) = 0;
virtual bool getNumericalValue(const std::string& strValue, int& iValue) const = 0;
virtual bool getLiteralValue(int iValue, std::string& strValue) const = 0;
virtual bool addValuePair(unsigned int uiValue, const std::string& strValue) = 0;
virtual bool getNumericalValue(const std::string& strValue, unsigned int& uiValue) const = 0;
virtual bool getLiteralValue(unsigned int iValue, std::string& strValue) const = 0;
virtual bool isTypeInclusive() const = 0;
virtual std::string getFormattedState(int iValue) const = 0;
virtual std::string getFormattedState(unsigned int iValue) const = 0;

/** deprecated, use getNumericalValue(const std::string& unsigned int&) instead */
bool getNumericalValue(const std::string& strValue, int& iValue) const
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deprecated (doxygen syntax)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

{
return getNumericalValue(strValue, (unsigned int&)iValue);
};

protected:
virtual ~ISelectionCriterionTypeInterface() {}
Expand Down
Loading