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 incorrect heap size value for boxed slice #13

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CeleritasCelery
Copy link

Previously the GetSize impl for a boxed slice didn't have a bound on T, so get_size was being called on &T instead of T. This meant that the size of each element was always being calculated to the size of a reference. This change fixes that and adds a test.

Previously the `GetSize` impl for a boxed slice didn't have a bound on `T`, so
`get_size` was being called on `&T` instead of `T`. This meant that the size of
each element was always being calculated to the size of a reference. This change
fixes that and adds a test.
nicmue added a commit to everestsystems/get-size that referenced this pull request Oct 26, 2023
Adds the enhancements proposed by:
- DKerp/get-size#11
- DKerp/get-size#13

Also adds support for `uuid`, `dashmap` and `serde_json`.
@CeleritasCelery
Copy link
Author

@DKerp ping

@bircni
Copy link

bircni commented Sep 13, 2024

@CeleritasCelery Ping

@CeleritasCelery
Copy link
Author

Hey @bircni Thanks for taking ownership of this code! I am going to switch to using get-size2 since it has this fix among other improvements.

@bircni
Copy link

bircni commented Sep 14, 2024

No Problem @CeleritasCelery please feel free to raise other issues or pull requests !

@bircni
Copy link

bircni commented Sep 14, 2024

the next version I publish will include your changes!

@bircni
Copy link

bircni commented Sep 14, 2024

Also opened an issue here:
rustsec/advisory-db#2078

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.

2 participants