Skip to content

Conversation

@loximann
Copy link
Contributor

@loximann loximann commented Jun 2, 2022

Description

Running with --listeners without registered listeners caused Catch2 programs to crash.

$ ./a.out --list-listeners
zsh: segmentation fault (core dumped)  ./a.out --list-listeners

The reason was due to dereferencing an iterator pointing past the end of a vector when the vector is empty.

@codecov
Copy link

codecov bot commented Jun 2, 2022

Codecov Report

Merging #2442 (7ae9ffc) into devel (cca5923) will decrease coverage by 0.01%.
The diff coverage is 66.67%.

❗ Current head 7ae9ffc differs from pull request most recent head 0057eb1. Consider uploading reports for the commit 0057eb1 to get more accurate results

@@            Coverage Diff             @@
##            devel    #2442      +/-   ##
==========================================
- Coverage   91.43%   91.42%   -0.01%     
==========================================
  Files         159      159              
  Lines        7501     7503       +2     
==========================================
+ Hits         6858     6859       +1     
- Misses        643      644       +1     

@loximann
Copy link
Contributor Author

loximann commented Jun 2, 2022

Should I add a test case to cover listing when no listeners are registered?

@horenmar
Copy link
Member

horenmar commented Jun 2, 2022

Should I add a test case to cover listing when no listeners are registered?

Yes.

@horenmar horenmar added the BugFix label Jun 2, 2022
@loximann loximann force-pushed the fix_list_listeners_crash branch from 7ae9ffc to 9bf2eae Compare June 2, 2022 11:47
@loximann
Copy link
Contributor Author

loximann commented Jun 2, 2022

I modified the fix so that defaultListListeners is single return (which in general I think makes things cleaner). With that, no test cases are needed for full coverage; however I added a test case to avoid regressions.

@horenmar
Copy link
Member

horenmar commented Jun 2, 2022

Please keep the early exit.

And coverage isn't really important, the regression check is.

@loximann loximann force-pushed the fix_list_listeners_crash branch from 9bf2eae to 0057eb1 Compare June 2, 2022 11:58
@horenmar horenmar merged commit 5efd327 into catchorg:devel Jun 2, 2022
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.

2 participants