ezrosent / allocators-rs

Allocators in Rust
Apache License 2.0
311 stars 28 forks source link

Make bsalloc propagate allocation failures from mmap-alloc #76

Closed whentze closed 7 years ago

whentze commented 7 years ago

Instead of always expect-ing a successful allocation from mmap-alloc, propagate AllocErrs up to the user.

Fixes #4

ezrosent commented 7 years ago

Thanks for the PR. This is a good start, I just have a couple comments:

  1. Could you rephrase the commit message to something like "bsalloc: Propagate allocation failures from mmap-alloc" for consistency with other project contributions.
  2. You currently only account for allocation failures for the "large" allocations that hit bsalloc. I think it is worth making a corresponding change in the macro-generated allocators as well. This means modifying the unsafe fn alloc declaration in the sized_cache macro, and getting read of the .expect call to mmap. Let me know if you'd like more direct pointers to that code.
whentze commented 7 years ago

You're right, I missed that!

It should be fine now. I changed the message as well.

joshlf commented 7 years ago

Hi @whentze , thanks for the PR! I'll let @ezrosent sign off on these changes, but could you add an entry in bsalloc's CHANGELOG.md to reflect these changes? Thanks!

If you want ideas, something like the following would work:

### Changed
- Changed allocation routines so that allocation failure is properly reported