New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MTS targets: Don't use any printf() calls when NDEBUG is defined #5013

Merged
merged 1 commit into from Sep 5, 2017

Conversation

Projects
None yet
7 participants
@janjongboom
Contributor

janjongboom commented Sep 4, 2017

The current behavior prevents linking out printf() and friends in release builds. This patch makes blinky on xDot use 15K(!!!!) less flash when compiling release build with GCC_ARM.

/cc @chrissnow This pushes my bootloader for xDot to 22K on GCC, 21K on ARMCC.

@reissjason @sg-

MTS targets: Don't use any printf() calls when NDEBUG is defined, pre…
…vents linking out printf() and friends in release build
@bulislaw

It make sense to remove prints in release, but why do you add new macro in one place and just remove the printf in another?

@janjongboom

This comment has been minimized.

Contributor

janjongboom commented Sep 4, 2017

@bulislaw Because in 2 files there's only one printf so wrapping it in an #ifdef made sense. In the other file there's multiple calls to printf so figured I'd macro it up.

I'm open to suggestions though on how to make this more clean.

@bulislaw

This comment has been minimized.

Member

bulislaw commented Sep 4, 2017

Fair enough. I guess we should introduce some sort of mbed wide debug print wrappers (that don't require external lib) sooner rather than later.

@0xc0170

0xc0170 approved these changes Sep 4, 2017

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Sep 4, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 4, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Sep 5, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1210

All builds and test passed!

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Sep 5, 2017

@theotherjimmy theotherjimmy merged commit 5f9d984 into ARMmbed:master Sep 5, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sg-

This comment has been minimized.

Member

sg- commented Sep 5, 2017

Why didn't this use the debug statement?

https://github.com/ARMmbed/mbed-os/blob/master/platform/mbed_debug.h

@maclobdell

This comment has been minimized.

Contributor

maclobdell commented Sep 5, 2017

@0xc0170 0xc0170 self-assigned this Sep 6, 2017

0xc0170 added a commit to 0xc0170/mbed-os that referenced this pull request Sep 6, 2017

mts targets: fix debug() usage
Instead of using #if NDEBUG, we can directly invoke debug() function. Fixes ARMmbed#5013

adbridge added a commit that referenced this pull request Oct 6, 2017

mts targets: fix debug() usage
Instead of using #if NDEBUG, we can directly invoke debug() function. Fixes #5013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment