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

Macro expansion results in a constant boolean expression #6613

Merged
merged 1 commit into from Apr 16, 2018

Conversation

Projects
None yet
5 participants
@pauluap

pauluap commented Apr 11, 2018

Description

In file included from .././features/FEATURE_LWIP/lwip-interface/lwip/src/include/lwip/ip_addr.h:43:0,
                 from .././features/FEATURE_LWIP/lwip-interface/lwip/src/include/lwip/netif.h:46,
                 from ../features/FEATURE_LWIP/lwip-interface/lwip_stack.h:23,
                 from ../features/FEATURE_LWIP/lwip-interface/lwip_stack.c:23:
../features/FEATURE_LWIP/lwip-interface/lwip_stack.c: In function 'mbed_lwip_socket_bind':
.././features/FEATURE_LWIP/lwip-interface/lwip/src/include/lwip/ip4_addr.h:171:40: warning: the comparison will always evaluate as 'false' for the address of 'ip_addr' will never be NULL [-Waddress]
 #define ip4_addr_isany(addr1) ((addr1) == NULL || ip4_addr_isany_val(*(addr1)))
                                        ^
.././features/FEATURE_LWIP/lwip-interface/lwip/src/include/lwip/ip_addr.h:272:49: note: in expansion of macro 'ip4_addr_isany'
 #define ip_addr_isany(ipaddr)                   ip4_addr_isany(ipaddr)
                                                 ^~~~~~~~~~~~~~
../features/FEATURE_LWIP/lwip-interface/lwip_stack.c:1138:10: note: in expansion of macro 'ip_addr_isany'
     if (!ip_addr_isany(&ip_addr) && !mbed_lwip_is_local_addr(&ip_addr)) {
          ^~~~~~~~~~~~~

Because the function ip_addr_isany() is actually a macro, it ends up being expanded in place with a constant boolean result.

My solution is a dirty trick, providing a pointer variable alias. My thought that is the ideal solution would use a function rather than a macro, but there's probably other considerations

Pull request type

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@pauluap pauluap force-pushed the pauluap:compiler_warning_macro_expansion_constant_boolean branch from 460a760 to 090647e Apr 12, 2018

@pauluap pauluap changed the base branch from mbed-os-5.8 to master Apr 12, 2018

@0xc0170 0xc0170 requested a review from kjbracey-arm Apr 13, 2018

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Apr 13, 2018

I think the correct and optimised fix is to use ipaddr_is_any_val(ip_addr). Can you try that?

@0xc0170 0xc0170 added needs: work and removed needs: review labels Apr 13, 2018

@pauluap

This comment has been minimized.

pauluap commented Apr 13, 2018

Yes, replacing with ip_addr_isany_val is still successful in suppression of the compiler warnings

@kjbracey-arm

Fine, but please squash to one commit and get rid of all the unconnected formatting changes. We've got a big merge coming in from feature-emac, and I want to minimise conflicting changes to these areas.

Paul Thompson

@pauluap pauluap force-pushed the pauluap:compiler_warning_macro_expansion_constant_boolean branch from 9b15996 to 8e76150 Apr 13, 2018

@pauluap

This comment has been minimized.

pauluap commented Apr 13, 2018

Cool. Done.

@cmonr

cmonr approved these changes Apr 13, 2018

@cmonr cmonr added needs: CI and removed needs: work labels Apr 13, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Apr 13, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 13, 2018

Build : SUCCESS

Build number : 1748
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6613/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 16, 2018

/morph mbed2-build

@0xc0170 0xc0170 added needs: CI and removed ready for merge labels Apr 16, 2018

@cmonr cmonr added ready for merge and removed needs: CI labels Apr 16, 2018

@cmonr cmonr merged commit c867934 into ARMmbed:master Apr 16, 2018

12 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 8644 cycles (-316 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 10092B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label Apr 16, 2018

@pauluap pauluap deleted the pauluap:compiler_warning_macro_expansion_constant_boolean branch Apr 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment