umm_malloc
umm_malloc copied to clipboard
rhempel
There is no check if heap size is within configured limits
According to the documentation, max heap size = block_size * uint15 max, so for 8-byte blocks (by default), it's:
8 * 32767 = 2621424 B
This limitation is not mentioned in the "configuration" section of the README. I would consider this information important enough to mention there, but even more important would be adding a runtime check to verify if the numbers align.
I would consider adding an assert inside umm_init_heap() to check if requested size of the heap is within limits.
@rhempel what do you think about it? I can submit a PR if you agree it makes sense.
In the README file we have this paragraph…
Each memory block holds 8 bytes, and there are up to 32767 blocks available, for about 256K of heap space. If that's not enough, you can always add more data bytes to the body of the memory block at the expense of free block size overhead.
… is that what you are looking for?
Sure, that's what I mean, but this information if quite hidden (in the "Background" section). And a bigger issue to me is that the heap size is never verified at runtime - if you initialize the heap with a bigger size, it seems to work at the first glance, but has some unexpected side effects later. To avoid unpleasant surprises, I think it's better to add a check and inform the user as early as possible that they got the size wrong.
The change I'm talking about is actually fairly small, so I just prepared the PR to show you what I meant ;) see: https://github.com/rhempel/umm_malloc/pull/60
For what it's worth, this also bit me for a while as I tried to initialize with too much memory...
The segfault occurred at the last statement in umm_init_heap when it attempted to write a value just before what I passed in as ptr, the beginning of the heap.
As a newcomer to this library, it would have been helpful to me if umm_init_heap documented it's assumptions/requirements inline with where it is defined, perhaps similar to this nearby function.
Once I got past that, I started to really enjoy this library. So thank you!
To clarify, I'm in favor of documentation update at the most. I would have to remove any runtime assertions in my use case.