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

Adding a defensive check against a null pointer for the current test suite #94

Merged
merged 1 commit into from
Oct 28, 2017

Conversation

Lectem
Copy link
Contributor

@Lectem Lectem commented Oct 17, 2017

Description

If no test suite has been set and we fail in a test, then tc.m_test_suite is nullptr and will crash the program.

If no test suite has been set and we fail in a test, then `tc.m_test_suite` is `nullptr`.
@onqtam
Copy link
Member

onqtam commented Oct 18, 2017

Thats strange - can you provide more information on how to reproduce the problem?

There are many test cases that fail in the examples of the project (like here) and many of them are in no test suite. Also all examples are ran through the sanitizers, and the doctest header itself resets the test suite for each translation unit it is included in with this line.

merging this check is not a problem but I'm curious how it happened

@Lectem
Copy link
Contributor Author

Lectem commented Oct 21, 2017

Sorry I forgot to check how it happened, but my suspect might be precompiled headers or the fact no parameters at all were given to the context.
I'll check this at the beginning of the week.

@Lectem
Copy link
Contributor Author

Lectem commented Oct 25, 2017

OK so I found the issue, sorry for the delay.
It actually happens because doctest was included in the precompiled headers.
Other things might be broken too, but so far didn't see any other issue.

@onqtam
Copy link
Member

onqtam commented Oct 25, 2017

strange... I include doctest in my precompiled headers as well (clang/gcc/msvc latest versions)... Anyway - I'll merge this commit soon (not right now - started a contracting work a few days ago and theres a lot of work).

thanks for the PR!

@Lectem
Copy link
Contributor Author

Lectem commented Oct 25, 2017

No problem, I also find this strange. If I find more information about it I'll comment here

@onqtam onqtam merged commit e2c7f9b into doctest:dev Oct 28, 2017
@onqtam
Copy link
Member

onqtam commented Oct 28, 2017

decided to just merge this :)

@onqtam onqtam changed the title Fix nullptr access Adding a defensive check against a null pointer for the current test suite Oct 28, 2017
onqtam pushed a commit that referenced this pull request Oct 29, 2017
If no test suite has been set and we fail in a test, then `tc.m_test_suite` is `nullptr`.
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.

2 participants