tass-belgium / picotcp

PicoTCP is a free TCP/IP stack implementation
Other
1.19k stars 219 forks source link

memory manager not stand-alone #416

Open frederikvs opened 8 years ago

frederikvs commented 8 years ago

As mentioned in #371, we have a memory manager, but it still needs another system for providing it with fixed-size pages of memory. In most cases, this is solved by using an underlying malloc, but when your goal is to not use malloc at all, this doesn't make much sense. (For our tests, where the memory manager is mostly used today, this does make sense.)

The underlying system doesn't need to be full-blown memory manager, it only needs to hand out fixed-size blocks, and so can be far simpler.

There's a partial fix by @basilfx in #371, but it still uses string.h, stdio.h, some other stuff that's probably RIOT-only (bf_get_unset?). The interface there currently just uses a fixed number of pages - we should allow the user to decide the number of pages, or maybe even allow the user to give us the memory region, so that he has more control over where it's placed exactly. Before developing this, we need to decide what the interface looks like.

let's add this as a new module called e.g. pico_page_allocator.c, under its own #ifdefs and make options.

Off the top of my head, I see 3 options for the interface :

TODO

basilfx commented 8 years ago

I am willing to look into this. It's a good learning experience :-) My (partial) fix is indeed intended for RIOT-OS, but it is nothing more than keeping track of which pages have been allocated and which not.

I would vote for option two or three. I can imagine situations where an end-user wants to control where the memory is allocated. Or maybe a hybrid option, one for simplicity and one for more control?

S4mw1s3 commented 8 years ago

If I'm not mistaken, one should call void pico_mem_init(uint32_t memsize) before calling any other pico* function. We should also consider extending that one with e.g. a pointer to a (statically) pre-allocated memory block. Off course this would introduce an API change. Not sure if we want this. Although now might be a good point since the memory manager is not used that often (yet). I think it's worth considering.

frederikvs commented 8 years ago

@S4mw1s3 thanks for the input. In general I'm against modifying an existing API - we don't want to break existing external code - but adding a new function to the API is definitely possible. Something like pico_mem_init_static(uint8_t *buffer, uint32_t size) or so. This of course leads to a few more implementation questions : do we keep the bitfield of which pages are allocated in the buffer itself (a bit more complex, and we probably lose a whole page to that handful of bytes), or do we allocate some static memory to keep the bitfield in (maximum pages is fixed at compile time - have it depend on yet another #define?)