Mbed-os: Bug in mbed_alloc_wrappers.cpp

[DBS06]

Description of defect

Setting
"platform.heap-stats-enabled": false,
"platform.memory-tracing-enabled": false,
in mbed_app.json does not deactivate tracing in mbed_alloc_wrappers.cpp

Target(s) affected by this defect ?

every target

How is this defect reproduced ?

Set
"platform.heap-stats-enabled": false,
"platform.memory-tracing-enabled": false,
in mbed_app.json.
Compile with debug profile and set a break point i.e. in mbed_alloc_wrappers.cpp inside malloc_wrapper().

Solution:

All
#ifdef MBED_MEM_TRACING_ENABLED
and
#ifdef MBED_HEAP_STATS_ENABLED
should be replaced by:
#if MBED_MEM_TRACING_ENABLED
#if MBED_HEAP_STATS_ENABLED

[ciarmcom]

Internal Jira reference: https://jira.arm.com/browse/MBOTRIAGE-2411

[0xc0170]

Thanks for the report, we will review proposed fix.

[evedon]

These settings are disabled by default by being set to "null" not false see https://github.com/ARMmbed/mbed-os/blob/master/platform/mbed_lib.json/#L69-L115

[kjbracey-arm]

There is clearly a problem in the implementation. All boolean options should be settable to true or false.

This particular set of options has come up a number of times now, but we've not got around to fixing them.

Last time I talked about it was here: https://github.com/ARMmbed/mbed-os/issues/10987

[0xc0170]

These settings are disabled by default by being set to "null" not false see https://github.com/ARMmbed/mbed-os/blob/master/platform/mbed_lib.json/#L69-L115

Even with default ones, an app can do false and this should disable it in Mbed OS. It might be better if we start using false (also null supported) and do a check on its value rather.

Is this something we can clean up for 6?

[DBS06]

Even with default ones, an app can do false and this should disable it in Mbed OS. It might be better if we start using false (also null supported) and do a check on its value rather.

Is also my opinion.
Should I close this issue?

[evedon]

These settings are disabled by default by being set to "null" not false see https://github.com/ARMmbed/mbed-os/blob/master/platform/mbed_lib.json/#L69-L115

Even with default ones, an app can do false and this should disable it in Mbed OS. It might be better if we start using false (also null supported) and do a check on its value rather.

Is this something we can clean up for 6?

I agree that this is confusing and needs to be cleaned up.
If we want to maintain backwards compatibility then these macros will need #ifdef as well.

[kjbracey-arm]

If we want to maintain backwards compatibility then these macros will need #ifdef as well.

I said that myself in the issue I linked to above, but now I can't think why I said that.

Just #if MBED_CONF_XXX should be fine - that is treated as #if 0 if it's not defined (ie null in the JSON).

But maybe I've forgotten a subtlety.

[0xc0170]

PR fixing it was merged, I'll close this one.