Enable minimal-printf by default for all builds

[bulislaw]

Summary of changes

This PR enables minimal-printf library for all builds. That means that all calls to printf family (including snprintf) will be handled by the minimal-printf library. Main reason for the change are substantial ROM savings that can reach up to 20k. The below table shows result for simple RTOS blinky example in two variants (explicitly calling printf and without) for two boards and three profiles:

Impact of changes

Minimal-printf library supports most of the basic usecases:

* %d: signed integer [h, hh, (none), l, ll, z, j, t].
* %i: signed integer [h, hh, (none), l, ll, z, j, t].
* %u: unsigned integer [h, hh, (none), l, ll, z, j, t].
* %x: unsigned integer [h, hh, (none), l, ll, z, j, t], printed as hexadecimal number (e.g., ff).
* %X: unsigned integer [h, hh, (none), l, ll, z, j, t], printed as hexadecimal number (e.g., FF).
* %f: floating point (enabled by default).
* %F: floating point (enabled by default, treated as %f).
* %g: floating point (enabled by default, treated as %f).
* %G: floating point (enabled by default, treated as %f).
* %c: character.
* %s: string.
* %p: pointer (e.g. 0x00123456).

If your application requires more advanced functionality you'll have to revert to using standard library version.

Migration actions required

If your application requires advance features of printf family calls, you'll either simplify it or switch to the standard C library version. To do so modify your application configuration in mbed_app.json file to override the parameter target.printf_lib with the value std as shown below:

    "target_overrides": {
        "*": {
            "target.printf_lib": "std"
        }
    }

Documentation

No new documentation necessary. Generic heads up will be part of Mbed 6 blog post series.

---

Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[x] Major update (Breaking change E.g. Return code change / API behaviour change)

---

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

---

Reviewers

@evedon @hugueskamba @kjbracey-arm @AnttiKauppila

---

[ciarmcom]

@bulislaw, thank you for your changes.
@evedon @kjbracey-arm @AnttiKauppila @hugueskamba @ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-core please review.

[kjbracey]

Minimal-printf library supports most of the basic usecases:

Can you expand that to cover what it does and doesn't do with regard to field width, precision, zero pad and other auxiliary bits?

We need to audit the complete Mbed OS code-base to check it doesn't use anything beyond minimal-printf functionality anywhere itself (or extend minimal printf to handle whatever it is).

(Perhaps add assert traps inside minimal printf for debug/develop mode to catch unknown format options?)

You could maybe get away with not doing that when it was off by default, but there's no excuse if it's the default. (Or add an interlock to ensure you get an error if compiling a feature/component that needs full).

[bulislaw]

I went through our code base, looking at which features of printf are used in our code base. The most used unsupported feature seems to be field width (eg %5d) and it's usually use to pritify the debug output, rather than actual funcional use.
The only place where I am worried is x509 cert handling in TLS (eg x509.c and x509-cert.c) @Patater A bit of context, I'm trying to replace our std lib printf family calls with minimal-printf by default. It offers substantial flash savings, but doesn't offer all the functionality. Onew of the things it doesn't support is filed width and x509 code uses it a lot. Could you have a look at how you use the field length and whether if suddenly the output changed a bit (no 0 padding) everything would explode? Also are we writing out the certificates in mbed?

I also explored adding asserts or warnings where the minimal-printf encounters specifier it doesn't recognize. Asserts wouldn't probably be a good idea as it would be impossible to compile the code in debug mode. While runtime warnings are possible it would be very verbose and I don't think it's the right direction.

The options we have:

• Promote minimal-printf as an optional optimisation - it's basically what we do now, it generally works, but it's pushing all the responsibility to our users. We historically didn't guarantee that it will work with Mbed OS....
• Switch minimal-printf on by default - Mbed OS may be the "IoT OS" and we are not only targetting hard core embedded developers, but in the ned we are still working in constrained embedded environments. We should encounter good practices and educate developers that there are things they might not need and that fancy functionality has its costs. In the future we could push for smaller C libraries.
• Enabled by default in Mbed Baremetal mode - That would promote baremetal mode as a sort of pro mode that focuses on size a lot. That being said baremetal has quite big limitations right now, so that would need to be addressed.

Thoughts?

[Patater]

@bulislaw

The only place where I am worried is x509 cert handling in TLS (eg x509.c and x509-cert.c)

The snprintf code there seems only to be involved in pretty printing what is read from certificates, so I believe we are OK there. We may have to adapt some test code which parses the pretty printed output, but that shouldn't have a huge impact.

[mergify]

This PR cannot be merged due to conflicts. Please rebase to resolve them.

[bulislaw]

I rebased the branch on master.
Any thoughts on the change in general and my last comment?

[adbridge]

@kjbracey-arm any further comments on this ?

[ladislas]

Any thoughts on the change in general and my last comment?

@bulislaw from an outsider's perspective, I'm in favor of Switch[ing] minimal-printf on by default. I agree with the good practice and that shiny things are more expensive to use but not always (or always not?) needed.

[bulislaw]

@kjbracey-arm are you ok with this change as it is (especially the reply about asserts)?

@ARMmbed/mbed-os-maintainers Could we run nightlies on this branch?
@yogpan01 @JanneKiiskila could you guys run extended client test on this branch?
@mikter @SeppoTakalo @AnttiKauppila could you guys run any tests for your components that are not part of the nightly mbed runs on this branch please

[JanneKiiskila]

We should addnote that ll -variants are ONLY supported if minimal-printf-enable-64-bit is enabled. We've already had our run with this issue in the client side. LwM2M specification states that int == int64_t...

Also, in that case it failed and printed just lld, would be nice and more convenient to spot the issue if the % sign would have also been left there...

[bulislaw]

ping

[0xc0170]

CI started

[mbed-ci]

Test run: FAILED

Summary: 3 of 8 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

• jenkins-ci/mbed-os-ci_exporter
• jenkins-ci/mbed-os-ci_cloud-client-pytest
• jenkins-ci/mbed-os-ci_greentea-test

Pull request has been modified.

[mergify]

This PR cannot be merged due to conflicts. Please rebase to resolve them.

[bulislaw]

Test kicked after Evelyne fixes.

[mbed-ci]

Test run: FAILED

Summary: 1 of 7 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

• jenkins-ci/mbed-os-ci_greentea-test

[evedon]

CI started

[mbed-ci]

Test run: FAILED

Summary: 2 of 7 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

• jenkins-ci/mbed-os-ci_dynamic-memory-usage
• jenkins-ci/mbed-os-ci_cloud-client-pytest

[0xc0170]

both tests restarted

[evedon]

Ready for maintainers approval.

[0xc0170]

Is this considered breaking change, I would say yes (I added a label) .

[0xc0170]

@kjbracey-arm @AnttiKauppila @hugueskamba Any more reviews? We should aim to integrate this early this week