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

Rework memory allocation failure handling in bytestring builders, add tests #242

Merged
merged 12 commits into from
Dec 27, 2022

Conversation

PJK
Copy link
Owner

@PJK PJK commented Dec 27, 2022

Description

The decoder should clean up intermediate items when the callback fails. The rest of the cleanup is handled by the decoder stack unwinding

Replaces #228. In retrospect, the fix wasn't a good solution: the new behavior did fix the memory leak in the streaming decoder, but the semantics didn't make sense for standalone usage of cbor_bytestring_add_chunk and were also inconsistent with cbor_array_push.

Fixes #231

Checklist

  • I have read followed CONTRIBUTING.md
    • I have added tests
    • I have updated the documentation
    • I have updated the CHANGELOG
  • Are there any breaking changes?
    • If yes: I have marked them in the CHANGELOG (example)
  • Does this PR introduce any platform specific code?
  • Security: Does this PR potentially affect security?
  • Performance: Does this PR potentially affect performance?

@codecov-commenter
Copy link

codecov-commenter commented Dec 27, 2022

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.49%. Comparing base (118f8d3) to head (b7c04c2).
Report is 271 commits behind head on master.

Files with missing lines Patch % Lines
src/cbor/internal/builder_callbacks.c 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #242      +/-   ##
==========================================
+ Coverage   95.85%   96.49%   +0.64%     
==========================================
  Files          20       20              
  Lines        1544     1541       -3     
==========================================
+ Hits         1480     1487       +7     
+ Misses         64       54      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@PJK PJK marked this pull request as ready for review December 27, 2022 17:54
@PJK PJK force-pushed the builder-callback-bytestring branch from 23c2d7c to b7c04c2 Compare December 27, 2022 18:03
@PJK PJK merged commit 1bca72c into master Dec 27, 2022
@PJK
Copy link
Owner Author

PJK commented Dec 27, 2022

cc @James-ZHANG

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.

Improve test coverage of cbor_bytestring_add_chunk failure scenarios
2 participants