PelionIoT / mbed-trace

mbed trace library
Apache License 2.0
18 stars 15 forks source link

Allow selecting the used memory allocation implementation at build time. #26

Closed tommikas closed 8 years ago

tommikas commented 8 years ago

Allows configuring the used implementation with a config.json entry of the form:

  "mbed-trace": {
    "mem": {
      "include": "\"my_header.h\"",  
      "alloc": "my_malloc",
      "free": "my_free"    
    }
  }

or by the usual means of passing in preprocessor directives in C builds using the definitions

MBED_TRACE_MEM_INCLUDE
MBED_TRACE_MEM_ALLOC
MBED_TRACE_MEM_FREE 

If nothing is configured, defaults to stdlib.

tommikas commented 8 years ago

@jupe How does this look? The mbed-trace-PR/label=linux,target=x86-linux-native build seems to be failing due to coverage again, but it actually seems unchanged looking at lcov->genhtml output. Does the coverage limit increase in ARMmbed/mbed-trace#25 already affect the whole repository?

@korjaa Fyi, this is what it looks like at the moment.

yogpan01 commented 8 years ago

@jupe @tommikas Can this be handled differently and not by config.json ? Yotta is going away and this will soon be deprecated, we should move this logic to some header files.

tommikas commented 8 years ago

The yotta functionality is just one alternative at the moment. It doesn't need to be used, and it can be easily removed whenever without affecting the other functionality. Happy to leave it out if it's considered unnecessary/unwanted, though.

Non-yotta builds are accommodated through 3 definitions, which are effectively synonymous to the yotta configuration options:

MBED_TRACE_MEM_INCLUDE
MBED_TRACE_MEM_ALLOC
MBED_TRACE_MEM_FREE

They make it possible to define whatever implementation is desired, including nsdynmemLIB. If nsdynmemLIB (for example) is the only alternative to stdlib that anyone using this library might want to use, I can put that behind a single flag or something, with stdlib remaining the default option.

tommikas commented 8 years ago

@yogpan01 Did my previous comment address your concerns? I'm not sure if you saw it...

@korjaa Would this be sufficient for you? Even if you don't need to follow the master branch at the moment, might as well make it usable now rather than tinker with it later.

jupe commented 8 years ago

lgtm

korjaa commented 8 years ago

+1 looks good, cannot think any improvements at the moment.

yogpan01 commented 8 years ago

lgtm