Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CATValues == operator incorrect #20252

Closed
msandstedt opened this issue Jul 3, 2022 · 0 comments · Fixed by #20253
Closed

CATValues == operator incorrect #20252

msandstedt opened this issue Jul 3, 2022 · 0 comments · Fixed by #20253
Assignees
@msandstedt

Comments

@msandstedt
Copy link
Contributor

Problem

CATValues == operator considers equality by position in the contained values std::array. However, CATs are not positional.

Proposed Solution

Fix the == operator to consider CATValues equal if both a,b contain the same values in any order.

CC @kghost , @bzbarsky-apple
@msandstedt msandstedt self-assigned this Jul 3, 2022
@msandstedt msandstedt mentioned this issue Jul 3, 2022
Merged
msandstedt added a commit to msandstedt/connectedhomeip that referenced this issue Jul 3, 2022
@msandstedt
Fix CATValues == operator
0a9e1eb 
CATValues == was using std::array equality, but this considers order
of the data, which is not relevant for CATs.  Fix this by sorting first
and then comparing.

While we're at it, add != and < operators.  The latter is useful for
sorting (e.g. for maps and sets).

Fixes project-chip#20252

Testing: added unit tests for the new and corrected operators.
tcarmelveilleux added a commit that referenced this issue Jul 5, 2022
@msandstedt @tcarmelveilleux
Fix CATValues == operator (#20253)
13ac032 
* Fix CATValues == operator

CATValues == was using std::array equality, but this considers order
of the data, which is not relevant for CATs.  Fix this by sorting first
and then comparing.

While we're at it, add != and < operators.  The latter is useful for
sorting (e.g. for maps and sets).

Fixes #20252

Testing: added unit tests for the new and corrected operators.

* Update src/lib/core/tests/TestCATValues.cpp

Co-authored-by: Tennessee Carmel-Veilleux <[email protected]>

* Update src/lib/core/tests/TestCATValues.cpp

Co-authored-by: Tennessee Carmel-Veilleux <[email protected]>

* Update src/lib/core/tests/TestCATValues.cpp

Co-authored-by: Tennessee Carmel-Veilleux <[email protected]>

* Fix comparison algorithm to properly handled kUndefinedCAT and repeated values

* CATValues are more like a set: repeated values aren't relevant
* kUndefinedCAT especially must be ignored in comparison

Refactor the == operator to more precisely reflect the sementics of CATs
given the above.  This removes the need to instantiate temporary stack
objcts for sort; simple sorting won't work anyway.

* Fixes to CASEAuthTag for comparison

- Adds basic set operation of `Contains`, `ContainsIdentifier`
  and `GetNumTagsPresent`
- Add missing unit tests for existing operations

* restyle

* Update src/lib/core/CASEAuthTag.h

Co-authored-by: Tennessee Carmel-Veilleux <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@msandstedt msandstedt

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix CATValues == operator msandstedt/connectedhomeip
1 participant
@msandstedt