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: Avoid invalid memory access in the logger #310

Merged
merged 3 commits into from
Jun 19, 2020
Merged

Conversation

Swatinem
Copy link
Member

@Swatinem Swatinem commented Jun 19, 2020

This also turns on the logger for the HTTP-based tests.

Fixes #309

This also turns on the logger for the HTTP-based tests, and enables
more testing of the crashpad backend.
@daxpedda
Copy link
Contributor

Just tried it, works perfectly 👌.

Copy link
Member

@jan-auer jan-auer left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! This originated in my comment at #301 (comment), which was written from poor memory about the API without testing it myself.

Are there still tests running without logs? We need to ensure that both paths are properly tested.

Please separate out all unrelated changes to tests into a separate PR, and then let's get this released as 0.3.4 directly on top of 0.3.3. We'll not merge any further features until code coverage is in place, and we run all tests with memory/address sanitizers.

@Swatinem
Copy link
Member Author

Are there still tests running without logs? We need to ensure that both paths are properly tested.

we test both with the standard http transport, with logs, and we also test with a specific stdout transport without logs, since I think that interferes with the way we capture the output, although stderr should be separate.

@jan-auer jan-auer changed the title fix: Avoid unsafety in the logger fix: Avoid invalid memory access in the logger Jun 19, 2020
@jan-auer jan-auer merged commit 4360d40 into master Jun 19, 2020
@jan-auer jan-auer deleted the fix/logger-testing branch June 19, 2020 18:51
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.

set_debug(true) crashes
3 participants