Conversation
foryoung365
commented
Oct 21, 2024
- Fix the null pointer exception issue in Tokenizer::dump
- Fix the issue where when there is a header file with the same name in the current source code path and the include path, it is incorrectly matched to the header file in the include path
Fix the problem where when there is a header file with the same name in the current source code path and the include path, it is incorrectly matched to the header file in the include path
|
Thanks for your contribution.
Also your existence check is flawed - see danmar/simplecpp#339. I will leave this open since you also made a change within the Cppcheck. Please also provide a test case to trigger the NULL pointer dereference. |
|
If supplying a PR you should also always add a test to make sure this behavior does not regress. Unfortunately we currently do not have a framework in simplecpp to do tests with includes - see danmar/simplecpp#362 (comment). |
|
The modification to Tokenizer::dump was triggered when dumping an unverified AST while locating the issue with the simplecpp header file inclusion. For the AST printed using the --dump parameter, I cannot reproduce it in my current usage scenario. Therefore, although I believe that always checking the validity of tok->scope() before use is a better practice, it currently does not seem to cause any issues, so I will close this PR. I will submit the PR for simplecpp to the simplecpp repo later. |
That is fair but as some recent fixes show we do rely on previous validation to hold up some assumptions. Thus the code lacks such checks in a lot of place and we usually only add those given a reproducer or static analysis warning.
Thanks. I hope to have a look at how to properly do tests with includes there soon. |