Skip to content

Fix some incorrect buffer sizes in the memory manager#2826

Merged
stevenengler merged 1 commit intoshadow:mainfrom
stevenengler:allocd-mem-size
Apr 3, 2023
Merged

Fix some incorrect buffer sizes in the memory manager#2826
stevenengler merged 1 commit intoshadow:mainfrom
stevenengler:allocd-mem-size

Conversation

@stevenengler
Copy link
Copy Markdown
Contributor

One fix is in a log statement, and the other is in AllocdMem which we only ever use as an AllocdMem<u8>, so neither of these currently cause incorrect behaviour.

@stevenengler stevenengler self-assigned this Apr 1, 2023
@github-actions github-actions bot added the Component: Main Composing the core Shadow executable label Apr 1, 2023
@stevenengler stevenengler changed the title Fixe some incorrect buffer sizes in the memory manager Fix some incorrect buffer sizes in the memory manager Apr 1, 2023
@stevenengler stevenengler requested a review from sporksmith April 3, 2023 14:03
Copy link
Copy Markdown
Contributor

@sporksmith sporksmith left a comment

Choose a reason for hiding this comment

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

Good catch!

Maybe we ought to add a method like byte_count?

Similarly I'm a little tempted to rename len to something like item_count. I think I originally used len to parallel the slice API, but in practice it's relatively common to want the length in bytes, and apparently easy to forget that len isn't that, at least for me :)

One fix is in a log statement, and the other is in `AllocdMem` which we only
ever use as a `AllocdMem<u8>`, so neither of these currently cause incorrect
behaviour.
@stevenengler stevenengler enabled auto-merge April 3, 2023 15:15
@stevenengler stevenengler merged commit 8e95682 into shadow:main Apr 3, 2023
@stevenengler stevenengler deleted the allocd-mem-size branch April 3, 2023 15:35
@stevenengler
Copy link
Copy Markdown
Contributor Author

Maybe we ought to add a method like byte_count?

Similarly I'm a little tempted to rename len to something like item_count. I think I originally used len to parallel the slice API, but in practice it's relatively common to want the length in bytes, and apparently easy to forget that len isn't that, at least for me :)

I think these would sound good to me. It's nice to follow the slice API, but yeah if it makes the code clearer then I think renaming them makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Main Composing the core Shadow executable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants