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

cleanup: Remove bin_pack_{new,free}. #2572

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

iphydf
Copy link
Member

@iphydf iphydf commented Jan 16, 2024

We should only ever use bin_pack_obj and friends, which stack-allocate the packer and pass it to callbacks.


This change is Reviewable

@iphydf iphydf added this to the v0.2.19 milestone Jan 16, 2024
@iphydf iphydf marked this pull request as ready for review January 16, 2024 17:33
Copy link

codecov bot commented Jan 16, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (21a8ff5) 66.96% compared to head (8109291) 66.94%.

❗ Current head 8109291 differs from pull request most recent head 259de48. Consider uploading reports for the commit 259de48 to get more accurate results

Files Patch % Lines
toxcore/tox_events.c 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2572      +/-   ##
==========================================
- Coverage   66.96%   66.94%   -0.03%     
==========================================
  Files         148      148              
  Lines       30275    30261      -14     
==========================================
- Hits        20275    20257      -18     
- Misses      10000    10004       +4     

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

@iphydf iphydf force-pushed the bracket-only branch 4 times, most recently from d8eba15 to a950417 Compare January 16, 2024 18:11
Copy link
Member

@JFreegman JFreegman left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 3 files at r1, 8 of 8 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

We should only ever use `bin_pack_obj` and friends, which stack-allocate
the packer and pass it to callbacks.
@nurupo
Copy link
Member

nurupo commented Jan 16, 2024

Interesting, so instead of alloc, call, dealloc, we just pass a callback into a function that does stack allocation and does the call.

@toktok-releaser toktok-releaser merged commit 259de48 into TokTok:master Jan 16, 2024
54 checks passed
@iphydf iphydf deleted the bracket-only branch January 16, 2024 19:27
This pull request was closed.
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.

4 participants