Skip to content

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Feb 12, 2021

Improve Iterator.qll models:

  • for the most part Iterator.qll already models its classes, and uses a special predicate getIteratorArgumentInput to connect classes to non-member function models. But there were a few opportunities to clean things up for member function models using getClassAndName.
  • there were some issues with the operator+= models, particularly with the non-member version and consistency between the member and non-member versions; I've added some more test cases and attempted to improve matters. But please do say if you think I may have made a mistake!

Regarding BSL:

  • the bsl classes appear to have iterators, begin methods etc with the normal behaviour.
  • The bsl::iterator, bsl::front_inserter, bsl::inserter, and bsl::back_inserter functions don't appear to be documented but are implemented as references (via using declarations) to the std versions.
  • depending on macros there is a bsl:iterator_traits corresponding to std::iterator_traits, or a using. I couldn't find documentation for it but it's clearly intended to be the same thing.
  • there are also a handful of special iterators (I think), such as bsl::List_Iterator. This example should be caught by IteratorByTypedefs and modelled correctly.

TL;DR: bsl iterators work the same as std iterators, so I've changed the model to include them, and also fixed a few things.

@geoffw0 geoffw0 added the C++ label Feb 12, 2021
@geoffw0 geoffw0 requested a review from a team as a code owner February 12, 2021 16:15
@jbj jbj requested a review from rdmarsh2 February 15, 2021 10:31
criemen
criemen previously approved these changes Feb 15, 2021
Copy link
Collaborator

@criemen criemen left a comment

Choose a reason for hiding this comment

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

LGTM, but somebody else should double-check the new taint flow.

@jbj
Copy link
Contributor

jbj commented Feb 15, 2021

somebody else should double-check the new taint flow.

@rdmarsh2, can you check it?

Copy link
Collaborator

@criemen criemen left a comment

Choose a reason for hiding this comment

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

As Robert didn't complain about the new taint flow, and his comment has been addressed, LGTM.

@criemen criemen merged commit ebcecca into github:main Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants