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.

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

Fix out of bound access in CalendarTest::testDayLists #838

Merged
merged 3 commits into from
Jun 18, 2020

Conversation

leonardoarcari
Copy link
Contributor

This PR fixes an out of bound bug in CalendarTest::testDayLists.

Basically, no iterator-end-check was performed in

if(d == *it_holidays && d == *it_businessDays) {

Therefore, when either it_holidays or it_businessDays reach end iterator, they're still dereferenced to be checked against d, hence yielding to undefined behavior.

This was detected by MSVC when compiling test-suite in debug mode.

@boring-cyborg
Copy link

boring-cyborg bot commented Jun 16, 2020

Thanks for opening this pull request! It might take a while before we look at it, so don't worry if there seems to be no feedback. We'll get to it.

@CLAassistant
Copy link

CLAassistant commented Jun 16, 2020

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

coveralls commented Jun 16, 2020

Coverage Status

Coverage remained the same at 71.071% when pulling 0128feb on leonardoarcari:master into e6e81b2 on lballabio:master.

@lballabio
Copy link
Owner

Hello! A suggestion: to keep the PR insulated from further changes you might make in your repo, you might consider opening it from a branch, and not from master. This way, further commits you do on master will not appear here.

@leonardoarcari
Copy link
Contributor Author

Hello! A suggestion: to keep the PR insulated from further changes you might make in your repo, you might consider opening it from a branch, and not from master. This way, further commits you do on master will not appear here.

Yeah! 😆

I realized that I messed up the second I pushed another commit. I reset master branch to the point I opened this PR and moved all other changes to a dedicated branch. Thanks!

(cherry picked from commit c0e8d4a)
@lballabio lballabio added this to the 1.19 release milestone Jun 17, 2020
@lballabio lballabio merged commit 992f013 into lballabio:master Jun 18, 2020
@boring-cyborg
Copy link

boring-cyborg bot commented Jun 18, 2020

Congratulations on your first merged pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants