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

Conversation

sguiriec
Copy link

@sguiriec sguiriec commented Sep 28, 2016

In case XML data for Min and Max values are wrongly set (out of range). The
Import function is not checking out of range format. In order to avoid
roll over some additional boundary check need to be done.

For exemple if we have Sign integer on 8 bits and we set Max value to 200
the new code will initialize the Max value to 127.

Signed-off-by: Sebastien Guiriec sebastien.guiriec@intel.com

@@             master       #375   diff @@
==========================================
Files           211        211
Lines          6683       6699    +16
Methods        1438       1439     +1
Messages          0          0
Branches        857        859     +2
==========================================
+ Hits           4814       4826    +12
- Misses         1397       1399     +2
- Partials        472        474     +2

Powered by Codecov. Last update b871a8f...ba4e6ae

if (condition) {
statement;
}
- [ ] <a href='#crh-comment-Pull 14123028502e1d13cea32d3796fa240afe04478d parameter/IntegerParameterType.cpp 66'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/01org/parameter-framework/pull/375#discussion_r81114649'>File: parameter/IntegerParameterType.cpp:L448-464</a></b>
- <a href='https://github.com/sguiriec'><img border=0 src='https://avatars.githubusercontent.com/u/124108?v=3' height=16 width=16'></a> As of today .xsd doesn't check the range. So this method allows to have same behavior as when we do not specify Min and Max.
In case of ERROR a tool should be provided in order to check XML mix/max range as I am not sure that the error will be explicit for a full platform XML loading.

  

In case XML data for Min and Max values are wrongly set (out of range). The
Import function is not checking out of range format. In order to avoid
roll over some additional boundary check need to be done.

For exemple if we have Sign integer on 8 bits and we set Max value to 200
the new code will initialize the Max value to 127.

Signed-off-by: Sebastien Guiriec <sebastien.guiriec@intel.com>
@sguiriec
Copy link
Author

@dwagner please have a look related to boundary check.

@sguiriec sguiriec closed this Sep 28, 2016
@sguiriec sguiriec reopened this Sep 28, 2016
This patch is fixing code formating according to clang

Signed-off-by: Sebastien Guiriec <sebastien.guiriec@intel.com>
Copy link
Contributor

@dawagner dawagner left a comment

Choose a reason for hiding this comment

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

Why don't we simply return an error when min/max values are incorrect?

type CIntegerParameterType::LimitValueAgainstRange(type value,
type minValue, type maxValue) const
{
if (value > maxValue) return(maxValue);
Copy link
Contributor

Choose a reason for hiding this comment

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

coding style:

if (condition) {
    statement;
}

Copy link
Author

Choose a reason for hiding this comment

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

As of today .xsd doesn't check the range. So this method allows to have same behavior as when we do not specify Min and Max.
In case of ERROR a tool should be provided in order to check XML mix/max range as I am not sure that the error will be explicit for a full platform XML loading.


// Limit Range accoridng to dynammic
template <typename type>
type CIntegerParameterType::LimitValueAgainstRange(type value,
Copy link
Contributor

Choose a reason for hiding this comment

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

coding style: method names start with a lowercase letter.

@codecov-io
Copy link

codecov-io commented Sep 29, 2016

Current coverage is 72.11% (diff: 88.46%)

Merging #375 into master will increase coverage by 0.07%

@@             master       #375   diff @@
==========================================
  Files           211        211          
  Lines          6683       6705    +22   
  Methods        1438       1439     +1   
  Messages          0          0          
  Branches        857        863     +6   
==========================================
+ Hits           4814       4835    +21   
+ Misses         1397       1396     -1   
- Partials        472        474     +2   

Powered by Codecov. Last update b871a8f...a7d3f0a

Sebastien Guiriec added 5 commits September 29, 2016 13:45
In case XML Min/Max values are not well set the import is returning
error.

Signed-off-by: Sebastien Guiriec <sebastien.guiriec@intel.com>
Minimum default value should be sign extended before boundary check.

Signed-off-by: Sebastien Guiriec <sebastien.guiriec@intel.com>
This patch is improving IntegerParameter Type unit tests in order
to check Min/max boundary when it is possible.

Signed-off-by: Sebastien Guiriec <sebastien.guiriec@intel.com>
Due to Min and Max format storage the Size of Integer parameter
should be limit to 32 bits.

Signed-off-by: Sebastien Guiriec <sebastien.guiriec@intel.com>
Enhance unit test in order to check data size above 32 bits.

Signed-off-by: Sebastien Guiriec <sebastien.guiriec@intel.com>
@dawagner dawagner mentioned this pull request Oct 4, 2016
2 tasks
@dawagner
Copy link
Contributor

I've just merged #376, which covers the same issues.

@dawagner dawagner closed this Oct 20, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants