Skip to content

chore: added check on iterator end position - #5129

Merged
henryiii merged 6 commits into
pybind:masterfrom
cliffburdick:issue_4822
May 28, 2024
Merged

chore: added check on iterator end position#5129
henryiii merged 6 commits into
pybind:masterfrom
cliffburdick:issue_4822

Conversation

@cliffburdick

@cliffburdickcliffburdick commented May 20, 2024•
edited by henryiii
Loading

Copy link
Copy Markdown
Contributor

Closes#4822

Description

Issue #4822 reported an issue Coverity found with an unchecked iterator in release mode. gcc13 also finds this by default and reports a warning.

Suggested changelog entry:

Avoid a warning by ensuring an iterator end check is included in release mode.

@cliffburdick

cliffburdick commented May 20, 2024

Copy link
Copy Markdown
ContributorAuthor

As far as I can tell the test that failed has nothing to do with this PR. Can anyone take a look?

Skylion007
Skylion007 reviewed May 24, 2024
View reviewed changes
Comment threadinclude/pybind11/detail/class.h Outdated
@henryiii

henryiii commented May 25, 2024

Copy link
Copy Markdown
Collaborator

Wouldn't it possibly be better to turn the assert into a failure that always triggers? Letting it fall through in release mode seems unideal, even though that's what's happening now.

@cliffburdick

cliffburdick commented May 26, 2024

Copy link
Copy Markdown
ContributorAuthor

Wouldn't it possibly be better to turn the assert into a failure that always triggers? Letting it fall through in release mode seems unideal, even though that's what's happening now.

Hi @henryiii , I refactored it to do what you suggested.

rwgk
rwgk requested changes May 27, 2024
View reviewed changes
Comment threadinclude/pybind11/detail/class.h Outdated
rwgk
rwgk requested changes May 27, 2024
View reviewed changes
Comment threadinclude/pybind11/detail/class.h Outdated
@cliffburdick
cliffburdick requested a review from rwgkMay 28, 2024 02:38
rwgk
rwgk approved these changes May 28, 2024
View reviewed changes

@rwgkrwgk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@henryiii
henryiii merged commit a5b9e50 into pybind:masterMay 28, 2024
@github-actionsgithub-actionsBot added the needs changelog Possibly needs a changelog entry label May 28, 2024
@henryiii

henryiii commented May 28, 2024

Copy link
Copy Markdown
Collaborator

Thanks for sticking with it!

@cliffburdick
cliffburdick deleted the issue_4822 branch May 28, 2024 14:39
@henryiiihenryiii changed the title Added check on iterator end positionchore: added check on iterator end positionJun 23, 2024
@henryiiihenryiii removed the needs changelog Possibly needs a changelog entry label Jun 26, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Skylion007Skylion007Skylion007 left review comments
@henryiiihenryiiihenryiii approved these changes
@rwgkrwgkrwgk approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG]: Coverity scan issue: Possible dereferencing iterator pos though it is already past the end of its container.

4 participants

@cliffburdick@henryiii@rwgk@Skylion007