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

Conversation

@dawagner
Copy link
Contributor

Using signed integers in the criterion API made it necessary to add guards for
undefined behaviour occuring when bit-shifting a signed integer up to its sign
bit. It also artifically reduced by 1 the number of possible inclusive values
in an inclusive criterion.

All APIs are changed in a retrocompatible fashion to take unsigned ints as
input parameters. However, getNumericalValue takes an output parameter. This
one must be kept as it is. We add a overloaded version of getNumericalValue
taking an unsigned int and depracate the "signed int" version.

Users that do not update their code to pass unsigned integers will still work
fine but will have the "31 inclusive criterion values" limitation.

- <a href='https://github.com/dawagner'><img border=0 src='https://avatars.githubusercontent.com/u/6430928?v=3' height=16 width=16'></a> done
- [x] <a href='#crh-comment-Pull c7c104d58ba25f0a16a7be6acf5e2d60d9b77bde parameter/include/SelectionCriterionTypeInterface.h 15'></a> <img src='http://www.codereviewhub.com/site/github-completed.png' height=16 width=60>&nbsp;<b><a href='https://github.com/01org/parameter-framework/pull/153#discussion_r35217805'>File: parameter/include/SelectionCriterionTypeInterface.h:L34-51</a></b>
- <a href='https://github.com/krocard'><img border=0 src='https://avatars.githubusercontent.com/u/6862950?v=3' height=16 width=16'></a> `@deprecated` (doxygen syntax)
- <a href='https://github.com/dawagner'><img border=0 src='https://avatars.githubusercontent.com/u/6430928?v=3' height=16 width=16'></a> done


<a href='https://www.codereviewhub.com/01org/parameter-framework/pull/153?mark_as_completed=1'><img src='http://www.codereviewhub.com/site/github-mark-as-completed.png' height=26></a>&nbsp;<a href='https://www.codereviewhub.com/01org/parameter-framework/pull/153?approve=1'><img src='http://www.codereviewhub.com/site/github-approve.png' height=26></a>&nbsp;<a href='https://github.com/01org/parameter-framework/pull/153'><img src='http://www.codereviewhub.com/site/github-refresh.png' height=26></a>
<a href='#crh-end'></a>

@dawagner
Copy link
Contributor Author

@krocard Please review

dawagner added 2 commits July 22, 2015 16:13
Using signed integers in the criterion API made it necessary to add guards for
undefined behaviour occuring when bit-shifting a signed integer up to its sign
bit. It also artifically reduced by 1 the number of possible inclusive values
in an inclusive criterion.

All APIs are changed in a retrocompatible fashion to take unsigned ints as
input parameters. However, getNumericalValue takes an output parameter. This
one must be kept as it is. We add a overloaded version of getNumericalValue
taking an unsigned int and depracate the "signed int" version.

Users that do not update their code to pass unsigned integers will still work
fine but will have the "31 inclusive criterion values" limitation.

Signed-off-by: David Wagner <[email protected]>
Otherwise, we'd trigger an undefined behaviour by shifting a unsigned int more
than its size.

Signed-off-by: David Wagner <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

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

do it here too (1U << uiState)

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.

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants