msp430: provide oneway-malloc implicitly#1182
msp430: provide oneway-malloc implicitly#1182Kijewski merged 3 commits intoRIOT-OS:masterfrom Kijewski:msp-oneway-malloc
Conversation
|
nice. I like the idea to exploit |
|
Nice one indeed, but please migrate the doxygen documentation as well. |
|
Doxygen added. |
|
The last commit brings |
|
Here comes your first ACK. Second needed because of (minor) core changes. |
cpu/msp430-common/oneway-malloc.c
Outdated
There was a problem hiding this comment.
I only moved the file.
|
Yeaa, this makes the core even smaller! I don't have the hardware, but I think this is PR is ok @rousselk do you like it? |
cpu/msp430-common/oneway-malloc.c
Outdated
There was a problem hiding this comment.
Is there such a Doxygen group?
|
this reduces the size of the text section for the hello world example from 51572 to 51556 : 16 bytes reduced |
Darn, I forgot to take the MSB-430 with me after the LNdW. |
I daresay that this is spurious, at lease it was not intended. It's just a residue of the alignment of the system. (Github recently shows my comments twice for me, and deletes both if I try to delete the extra one. -_- As I'm not going to try to clean up any probably non-existent mess anymore, please delete any duplicated comments of me if you cross some.) |
|
Hereby I revoke my former ACK (for now). Reason: The oneway malloc implementation might be useful for other platforms/toolchains than MSP430. |
|
@Kijewski the double comments disappear if you reload the page - its a JS bug I guess. |
|
It's now a sys module, and a default module for MSP boards. |
sys/oneway-malloc/oneway-malloc.c
Outdated
There was a problem hiding this comment.
You might want to change this.
There was a problem hiding this comment.
How about
@addtogroup oneway_malloc
@ingroup sys
?
There was a problem hiding this comment.
This should still be changed as the file is not in core anymore!
|
can we have a generic 'malloc' module and make all modules that need it depend on it? |
There was a problem hiding this comment.
Is there a reason, the header is not in $(RIOTBASE)/sys/include as all other sys-headers?
There was a problem hiding this comment.
I don't want to shadow the malloc.h of the libc. E.g. the function memalign is missing.
|
I think the PR is fine. I would also opt for enabling the So if the above issues are addressed I'd give it a go. |
|
@haukepetersen comments addressed. |
|
Please view new comments. |
|
copyright notice added. |
For MSP430 boards oneway-malloc is already used *if* `malloc.h` was included. The problem is that `malloc.h` is not a standard header, even though it is common. `stdlib.h` in the right place to look for `malloc()` and friends. This change removes this discrepancy. `malloc()` is just named like that, without the leading underscore. The symbols now are weak, which means that they won't override library functions if MSP's standard library will provide these functions at some point. (Unlikely, since using `malloc()` on tiny systems is less then optimal ...) Closes #1061 and #863.
Issue #1061 was fixed.
|
Doxygen group changed. |
|
Should be ready to merge. |
|
ACK. Maybe you can squash before you merge... |
msp430: provide oneway-malloc implicitly
8 participants
For MSP430 boards oneway-malloc is already used if
malloc.hwasincluded. The problem is that
malloc.his not a standard header, eventhough it is common.
stdlib.hin the right place to look formalloc()and friends.This change removes this discrepancy.
malloc()is just named likethat, without the leading underscore. The symbols now are weak, which
means that they won't override library functions if MSP's standard
library will provide these functions at some point. (Unlikely, since
using
malloc()on tiny systems is less then optimal ...)