Thu 18 Feb 2016 10:51:59 AM UTC, comment #3:
> I would have thought that both the mem heap and the memp pools go into bss unless you specially tell your linker to put them somewhere else.
Correct, but we have to put them somewhere else, specially the memory pools and heap, are put out of normal data and bss... A platform requirement.
> Also, as the memory block returned is not zeroed, zeroing the struct would not really catch all of these problems.
The memory block will be zeroed or written first by the user before it tries to read from it, but the user does it according to the alignment.
The metadata however, is not that aware of the alignment and byte enables, so doing a memset(mem, 0, sizeof(mem)); will take compiler alignment for that struct into account.
> It seems like this test is not a good one to enforce proper memory usage: we do use the memory correctly, but the platform fails in seeing this. What you see is a false positive. Plus this check only catches the first usage: once the memory was used, written and freed again, the check does nothing after next alloc.
Sorry, I did not want to imply that you were doing something wrong, but rather that our platform needs to take more things into account.
Having it that way catches reads before initialization, which is not a bad thing to have.
I'm just asking for an additional optional configuration option that will make mem and memp zeroize the metadata in the memory pools in a way that accounts for the structure current alignment.
> As such, I'm not convinced this changes helps much.
I agree that is not the usual case, but should not hurt to add something like:
#if LWIP_MEM[P]_FULLY_INITIALIZE_METADATA
memset(mem, 0, sizeof(mem));
#endif
or
#if LWIP_MEM[P]_FULLY_INITIALIZE_METADATA
memset(mem, 0, SIZEOF_STRUCT_MEM);
#endif
|