Add configuration option to disable printf entirely

[AGlass0fMilk]

Summary of changes

This PR adds a configuration option under the minimal-printf umbrella to simply replace all printf-like calls with no-ops essentially.

There is probably a cleaner way of doing this but I figured it could go under the existing minimal-printf overrides... it could be considered a minimal printf implementation (one that does nothing!)

This saves around 1kB of flash if printf is not required in your application. This is useful because it is not always feasible to simply comment out printf calls throughout your code, especially if you're using external libraries.

Impact of changes

Users wishing to completely disable printf can now do so by configuring platform.minimal-printf-disable-all to true.

Migration actions required

None

Documentation

Update minimal-printf documentation upon PR approval.

Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[X] Feature update (New feature / Functionality change / New API)
[] 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

---

[0Grit]

Thanks George. I have a need for this as well.

[ciarmcom]

@AGlass0fMilk, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

[hugueskamba]

Thanks for the suggestion.

If something like this was to be implemented, I would instead prefer it to be a possible option of the existing target.printf_lib.

[LDong-Arm]

Thanks @AGlass0fMilk.
I second what @hugueskamba said. The config target.printf defaults to minimal-printf and can be switch to std (standard printf from C libraries) currently, so another option (e.g. off) makes sense.

But lots of code (e.g. connectivity, storage in Mbed OS, many external libraries) use sprintf, snprintf, etc. to format data buffers, even if they don't print things to stdout. So we should be extra cautious when deciding whether to provide such an option, as users may not be fully aware. (I guess it's okay if we give a good notice in the documentation.)

[AGlass0fMilk]

@LDong-Arm @hugueskamba I agree with both statements. I did it this way to just get the PR up for discussion. I also wanted to avoid having to dive into the tools code, which I think will be necessary to add the target.printf = off option.

I think it'd be an acceptable option as long as we give fair warning in the documentation. My primary use case is for small Mbed-OS bootloaders that don't need to print anything and avoid using any variant of printf. Being able to save 1kB in this kind of build application is relatively significant. Without this option there's no way to completely eliminate the code bloat of printf without modifying library code, even if stdout is disabled.

Any suggestions on how to proceed with the target.printf = off implementation?

[hugueskamba]

@LDong-Arm @hugueskamba I agree with both statements. I did it this way to just get the PR up for discussion. I also wanted to avoid having to dive into the tools code, which I think will be necessary to add the target.printf = off option.

I think it'd be an acceptable option as long as we give fair warning in the documentation. My primary use case is for small Mbed-OS bootloaders that don't need to print anything and avoid using any variant of printf. Being able to save 1kB in this kind of build application is relatively significant. Without this option there's no way to completely eliminate the code bloat of printf without modifying library code, even if stdout is disabled.

Any suggestions on how to proceed with the target.printf = off implementation?

FYI @rwalton-arm (as it involves tools changes).

@AGlass0fMilk Are you aware of platform.stdio-minimal-console-only? It helps to reduce code size.

[0Grit]

@rwalton-arm how close are we to the new tools release? Should this just get implemented for the new stuff?

[LDong-Arm]

Well, the script to select printf is in the mbed-os repo itself.

mbed-os/tools/toolchains/mbed_toolchain.py

Lines 1086 to 1092 in e57e325

	def check_and_add_minimal_printf(self, target):
	"""Add toolchain flag if minimal-printf is selected."""
	if (
	getattr(target, "printf_lib", "std") == "minimal-printf"
	and "-DMBED_MINIMAL_PRINTF" not in self.flags["common"]
	):
	self.flags["common"].append("-DMBED_MINIMAL_PRINTF")

Once we have evaluated and agreed on the support for no-printf, we can make it by ourselves (both source code change and script change in one PR).

[adbridge]

@Patater @rwalton-arm I assume this would now need to wait until the new tools as we are freezing contributions to /tools in mbed-os ?

[Patater]

@Patater @rwalton-arm I assume this would now need to wait until the new tools as we are freezing contributions to /tools in mbed-os ?

@adbridge
The changes here look fine. No tools files are being modified that would affect the Online Compiler (and fail the new CI job #13349)

[Patater]

OK from tools side

[rwalton-arm]

@Patater @rwalton-arm I assume this would now need to wait until the new tools as we are freezing contributions to /tools in mbed-os ?

@adbridge
The changes here look fine. No tools files are being modified that would affect the Online Compiler (and fail the new CI job #13349)

The PR as it stands looks fine. However, if the suggestion of adding this as an extra target.printf_lib option to targets.json is preferred, that would require tools changes, which would affect the online compiler and fail the CI job.

[rwalton-arm]

@rwalton-arm how close are we to the new tools release? Should this just get implemented for the new stuff?

We're still a month or two away from the new tools release. We will need a way of handling this, preferably in CMake. We don't want logic to set build flags in the new Python tools.

[adbridge]

I would suggest parking this for now and making sure it is captured in the new tools. Otherwise we will get people used to doing things one way only to potentially have to change it again in a relatively short period ?

[AGlass0fMilk]

I agree with parking it. The implementation isn’t pretty and was a stopgap to get conversation started. Please capture this as a requirement for the updated tools. On Aug 7, 2020, at 8:30 AM, Anna Bridge <[email protected]<mailto:[email protected]>> wrote: I would suggest parking this for now and making sure it is captured in the new tools. Otherwise we will get people used to doing things one way only to potentially have to change it again in a relatively short period ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#13380 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AACVSJHH53IBP7Z4SKC35D3R7PXUPANCNFSM4POZOEDA>.

[adbridge]

@rwalton-arm @Patater could you please capture this as a requirement for the new tools work?

[Patater]

Printf library selection for the new tools is implemented and being added to Mbed OS at #13615

[0xc0170]

@AGlass0fMilk please look at CMake feature branch (it will be on master soon) and create a new issue how we can do this with CMake and new tools, thanks.

I'll close this.

[LDong-Arm]

Now I think an ideal way to disable printing (and save code size) is to use a trace library (e.g. mbed-trace) which, when disabled, removes trace statements with macros.
Even if we want to override minimal-printf instead, it makes sense to disable printf() but not sprintf(), etc.

[0Grit]

@LDong-Arm i agree, a robust trace system is the way to go.