Mbed-os: Behavior of boolean in mbed_app.json

[LaurentLouf]

Description

• Target : all
• Toolchain : all
• Tools : mbed-cli 1.10.0
• Mbed OS version : 5.11.1

The problem encountered concerns the mbed_app.json file and the way some (at least one) parameters are set. Sometimes, to save some time when I know that I'll need to toggle one parameter, I keep it in my mbed_app.json file, changing its value from true to false to activate/deactivate it. Problem is, for platform.memory-tracing-enabled (for example), doing this does not lead to the expected behavior. When setting the value to true all goes well, but setting it to false does not : the pre-processor constant exists but has a value of 0. Which means that the test #if defined(MBED_MEM_TRACING_ENABLED) is true, so we end up with the same behavior as when the value is set to true. To have the expected behavior, the value expected in mbed_app.json is null and not false.

In my humble opinion, setting it to null or false should lead to the same behavior.

Issue request type

[ ] Question
[X] Enhancement
[ ] Bug

[0xc0170]

You should always used if defined() first to check if value is defined, then the value check - this would overcome the issue about false vs null in the json. If this is a bug in our code, we should fix it.

To my understanding, true/false in the config leads to being the attribute defined with value specified, null to not be defined (not present in config header file). See https://os.mbed.com/docs/mbed-os/v5.13/reference/configuration.html for details.

We should allow both, either in this case memory-tracing-enabled is not defined (=disabled) or it's defined but disabled = same result in the final binary.

[LaurentLouf]

Yes, the current problem is that all tests use the same following structure (mbed_alloc_wrappers.cpp for example) :

#ifdef MBED_MEM_TRACING_ENABLED
    mbed_mem_trace_lock();
#endif

Pretty easy fix, but there's quite a lot of digging to find all tests to fix.

[ciarmcom]

Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-1414

[0xc0170]

cc @ARMmbed/mbed-os-core

[LaurentLouf]

Side note : in the documentation about memory tracing , it is said :

The memory tracer is not enabled by default. To enable it, you need to define the MBED_MEM_TRACING_ENABLED macro. The recommended way to define this macro is to add it to the list of macros defined in your mbed_app.json:

I remember being a bit confused when trying to decide whether to override some parameters defined in mbed_lib.json files : should I use "platform.memory-tracing-enabled": true, or the macros thing ? It feels more natural not to redefine a macro but instead define it correctly the first time, and leave the macro definition for macro that are not mbed_lib.json parameters.

[0xc0170]

That would explain why there is just ifdef 😕 Anyway, we shall fix this as we provide config.

[kjbracey-arm]

To my understanding, true/false in the config leads to being the attribute defined with value specified, null to not be defined (not present in config header file).

Correct.

Boolean Mbed configuation parameters should always be checked with #if MBED_CONF_XXX.

null and false do not mean the same thing in JSON. One is an explicit request for false, the other is leaving that parameter unset.

Putting null into any JSON configuration means you are not specifying that parameter - that permits the C code to have a default - possibly an intelligent default based on other settings.

You only need #ifdef MBED_CONF in the code if you do want to permit null in the JSON, or if you have code that may be compiling in a non-Mbed OS environment.

If a setting permits null to be set, it should usually document what null means - if it's a boolean or integer type thing. For some options it may be kind of obvious - eg for a password, null might mean no password, and hence security support disabled. For some like lwIP memory pools, null might mean "sensible default = 4 * MSS setting".

All those MBED_MEM_ macros are badly implemented, and should have been #if with false as the JSON defaults. Having an #ifdef as well seems like overkill, but I guess is needed now for backwards compatibility.

[ciarmcom]

Thank you for raising this issue. Please note we have updated our policies and
now only defects should be raised directly in GitHub. Going forward questions and
enhancements will be considered in our forums, https://forums.mbed.com/ . If this
issue is still relevant please re-raise it there.
This GitHub issue will now be closed.