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

Fix gcc [-Wsign-compare] warning #4192

Merged
merged 1 commit into from Apr 20, 2017

Conversation

Projects
None yet
7 participants
@fmanno
Contributor

fmanno commented Apr 17, 2017

Description

[Warning] mbed_board.c@99,36: comparison between signed and unsigned
integer expressions [-Wsign-compare] is seen during compilation.
Fix the warning and small improvements.

  1. Change type of loop variable "i" to the same type as "size".
    Size is > 0 checked by the if statement and the for loop stops at i == size
    so none can be negative. However size still needs to be signed
    to detect error codes from vsnprintf.
  2. Reduced scope of stdio_out_prev and make sure it's initialized.
  3. Define a name for the error buffer size and use vsnprintf instead
    of vsprintf to avoid writing outside of the array.
    NOTE: the if(size > 0) statement doesn't need to change. If
    the message to write is larger than the buffer size vsnprintf
    truncates it automatically but still returns a positive value.

Status

READY

Steps to test or reproduce

  1. mbed import mbed-os-example-blinky
  2. cd mbed-os-example-blinky
  3. mbed compile -m NUCLEO_F401RE
@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 18, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 18, 2017

/morph test

@fmanno

This comment has been minimized.

Contributor

fmanno commented Apr 18, 2017

I've signed the agreement at https://developer.mbed.org/contributor_agreement/ and linked my mbed profile to my github profile. Cheers!

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Apr 18, 2017

/morph test

@bulislaw

This comment has been minimized.

Member

bulislaw commented Apr 18, 2017

@studavekar @bridadan I think CI is not being triggered...

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Apr 18, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Apr 18, 2017

Result: FAILURE

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

/morph test

Output

mbed Build Number: 22

Example Build failed!

platform/mbed_board.c Outdated
if (size > 0) {
if (!stdio_uart_inited) {
serial_init(&stdio_uart, STDIO_UART_TX, STDIO_UART_RX);
}
#if MBED_CONF_PLATFORM_STDIO_CONVERT_NEWLINES
for (unsigned int i = 0; i < size; i++) {
for (char stdio_out_prev = '\0', int i = 0; i < size; i++) {

This comment has been minimized.

@bridadan

bridadan Apr 18, 2017

Contributor

This syntax is incorrect. Did you mean to put this outside of the loop?

@bulislaw @0xc0170 y'all took a look at this already, maybe you can make some suggestions.

This comment has been minimized.

@bulislaw

bulislaw Apr 18, 2017

Member

Ah, yes. I've missed that second definition... reverting to having stdio_out_prev defined outside of the loop should be ok.

This comment has been minimized.

@fmanno

fmanno Apr 18, 2017

Contributor

You're right, this should've been right before the loop not inside it.

@fmanno

This comment has been minimized.

Contributor

fmanno commented Apr 18, 2017

I've pushed a new commit to fix the declaration of stdio_out_prev. This time to test the change I've forced MBED_CONF_PLATFORM_STDIO_CONVERT_NEWLINES to be 1 to trigger the compile error first and later to check that the new code compiles.

@bridadan

Thanks for the changes!

@bridadan

This comment has been minimized.

Contributor

bridadan commented Apr 18, 2017

/morph test

@bridadan bridadan added needs: CI and removed needs: CLA labels Apr 18, 2017

@mbed-bot

This comment has been minimized.

mbed-bot commented Apr 19, 2017

Result: FAILURE

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

/morph test

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 19, 2017

Merge branch 'master' into improve-mbed_error_vprintf

Can you please rebase this branch to remove this merge commit?

Fix gcc [-Wsign-compare] warning
[Warning] mbed_board.c@99,36: comparison between signed and unsigned
integer expressions [-Wsign-compare] is seen during compilation.
Fix the warning and small improvements.

1. Change type of loop variable "i" to the same type as "size".
   Size is > 0 checked by the if statement and i stops at == size
   so none can be negative. However size still needs to be signed
   to detect error codes from vsnprintf.
2. Reduced scope of stdio_out_prev and make sure it's initialized.
3. Define a name for the error buffer size and use vsnprintf instead
   of vsprintf to avoid writing outside of the array.
   NOTE: the if(size > 0) statement doesn't need to change. If
   the message to write is larger than the buffer size vsnprintf
   truncates it automatically but still returns a positive value.

@fmanno fmanno force-pushed the fmanno:improve-mbed_error_vprintf branch to 4398b1f Apr 19, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 20, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Apr 20, 2017

Result: SUCCESS

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

/morph test

Output

mbed Build Number: 47

All builds and test passed!

@adbridge adbridge merged commit f1576bf into ARMmbed:master Apr 20, 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment