Makefile.cflags: Build with -Wall -Werror by default (including fixes to correct all existing warnings) by jnohlgard · Pull Request #3319 · RIOT-OS/RIOT

jnohlgard · 2015-07-05T22:58:26Z

Fixes a lot of warnings. This is a stepping stone towards #1121.

make buildtest with CFLAGS += -Wall -Werror (not -Wextra) added to Makefile.include now completes without errors on ~~examples/hello-world, examples/default, examples/rpl_udp, tests/unittests~~ all tests

Worth extra notice:

openmote GPIO periph_conf has been redefined to allow all pins as GPIOs (before only the few first were defined)
redbee-econotag does not set CFLAGS += -Wextra in the board Makefile anymore
msp430 now has a wrapper stdlib.h header for including malloc et al. as expected from <stdlib.h>

Kijewski · 2015-07-05T23:27:57Z

cpu/cortexm_common/cortexm_init.c

s/int i/IRQn_Type i/?

jnohlgard · 2015-07-06T10:41:05Z

Added -Wall -Werror as an experiment, waiting for Travis.

jnohlgard · 2015-07-06T23:06:52Z

Added a whole bunch of fixes.

Interesting parts:

msp430 getchar implementations for all msp430 boards except chronos
Blacklist MSP430 boards for libfixmath_unittests because of some missing math.h functions.
Blacklist MSP430 from CCN lite examples because the API seem to be designed with the assumption that int is larger than 16 bits wide.
Added libfixmath patches for fixing warnings inside the library.

jnohlgard · 2015-07-07T07:40:34Z

there's a problem with the qemu-i386 port and PRId64 being defined as d instead of lld because of a missing declaration of __have_long_long_64. I am guessing that a toolchain update might fix it but I am unfamiliar with the toolchain script.

OlegHahm · 2015-07-07T07:56:48Z

Awesome work! Thanks a lot!

I am guessing that a toolchain update might fix it but I am unfamiliar with the toolchain script.

Which toolchain script are you referring to?

jnohlgard · 2015-07-07T09:59:01Z

@OlegHahm I was referring to the script in dist/tools/toolchain/build_x86.sh.

boards/x86-multiboot-common/Makefile.include downloads a precompiled and patched newlib from http://download.riot-os.org/i586-newlib_2.1.0_tlsf-1254.tar.bz2. I am assuming this newlib was built via the build_x86.sh script.
The build_x86.sh script in turn has a bunch of cryptic patches without explanation what they are for or what they do, see https://github.com/RIOT-OS/RIOT/blob/master/dist/tools/toolchains/build_x86.sh#L30

I think the problem would be solved by an upgrade to newlib-2.2.0, which has some updates for the PRI* macros.

OlegHahm · 2015-07-07T10:31:57Z

@Kijewski, can you shed some light on this?

Kijewski · 2015-07-07T10:40:16Z

Yes, the changes are:

supplying my own malloc
un-inlining (f)putc
fixing some minor warnings

jnohlgard · 2015-07-07T11:09:07Z

nevermind my comment about newlib versions.
The real problem is that boards/x86-multiboot-common/Makefile.include is adding -isystem /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/include before the newlib includes, which means that my x86_64 libgcc stdint.h is picked instead of newlib's stdint.h. GCC does not supply the macros __have_long64 etc.

When supplying -ffreestanding as an argument to GCC, the compiler will define __STDC_HOSTED__ as 0 and include its own headers instead of the libc's (newlib's)

What is the correct procedure here? Should newlib's includes be added before GCC's on the command line? Is it possible to build a QEMU kernel without -ffreestanding?

jnohlgard · 2015-07-07T11:11:00Z

Right after I posted my last comment I found this mailing list post:

http://comments.gmane.org/gmane.comp.lib.newlib/9373

(The change discussed in the post is included in newlib 2.2.0, but not in newlib 2.1.0)

jnohlgard · 2015-07-07T11:38:42Z

Might need an even newer newlib:
https://sourceware.org/ml/newlib/2015/msg00406.html

jnohlgard · 2015-07-07T15:29:13Z

Rebased on latest master.

x86: Rename stack_t -> stack_info_t because recent newlib defines stack_t inside <sys/signal.h>.

See https://sourceware.org/ml/newlib-cvs/2015-q2/msg00001.html

The qemu-i386 tests work now if I replace the files in toolchain/x86/i586-none-eabi/include by the include files from my installed newlib (from git some weeks old). All PRId64, PRIu32 bugs have been fixed in newlib now.

OlegHahm · 2015-07-07T16:50:36Z

sys/net/include/etx_beaconing.h

Why not replacing with MS_IN_USEC while you're at it anyway?

Fixes warnings about wrong function signature.

jnohlgard · 2015-07-14T06:12:53Z

squashed, waiting for Travis

Makefile.cflags: Build with -Wall -Werror by default (including fixes to correct all existing warnings)

OlegHahm · 2015-07-14T08:16:30Z

Woohoo!

thomaseichinger · 2015-07-14T08:57:47Z

This broke building for OS X and FreeBSD. Again 😢

jnohlgard · 2015-07-14T09:37:37Z

This broke building for OS X and FreeBSD. Again 😢

Sorry about that!

jnohlgard · 2015-07-14T09:39:05Z

@thomaseichinger is FreeBSD and OSX sufficiently similar to assume that any build fixes for FreeBSD usually gets it working on OSX as well?

thomaseichinger · 2015-07-14T09:40:50Z

No, sadly not.

miri64 · 2015-07-14T09:42:23Z

It's about time, that Millhouse (Mac@FU) and Stedten (FreeBSD@FU) are included in the build tests :D

thomaseichinger · 2015-07-14T09:47:02Z

I fear we wont fix this for OS X, as the used ucontext API used by native is marked as deprecated.

jnohlgard · 2015-07-14T10:09:38Z

@thomaseichinger we could add an ifeq check for OSX in the Makefile and defer fixing this until the next OSX release where the deprecated stuff is removed.

thomaseichinger · 2015-07-14T10:10:03Z

I'm on it.

jnohlgard · 2015-07-14T10:11:25Z

@thomaseichinger does -Wno-error=deprecated get the correct result?

thomaseichinger · 2015-07-14T10:52:26Z

@gebart I went with -Wno-deprecated-declarations.

jnohlgard assigned OlegHahm Jul 5, 2015

Kijewski reviewed Jul 5, 2015
View reviewed changes

jnohlgard force-pushed the pr/warning-fixes-3 branch 5 times, most recently from 59e1aae to 414de24 Compare July 6, 2015 23:01

jnohlgard force-pushed the pr/warning-fixes-3 branch from 414de24 to 2618e64 Compare July 6, 2015 23:08

Kijewski added the PR-award-nominee label Jul 7, 2015

jnohlgard changed the title ~~More warning fixes~~ Makefile.cflags: Build with -Wall -Werror by default (including fixes to correct all existing warnings) Jul 7, 2015

jnohlgard force-pushed the pr/warning-fixes-3 branch from 2618e64 to 59668a1 Compare July 7, 2015 15:24

OlegHahm reviewed Jul 7, 2015
View reviewed changes

sys/net/include/etx_beaconing.h Outdated

Copy link
Copy Markdown

Member

OlegHahm Jul 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not replacing with MS_IN_USEC while you're at it anyway?

Joakim Gebart added 9 commits July 14, 2015 08:11

boards/msb-430-common: Add getchar implementation

5a99d5c

boards/wsn430-common: Add getchar implementation

1e63aa6

boards/z1: Add getchar implementation

68c47d2

tests/shell: Blacklist chronos for missing getchar implementation

b318d26

tests/slip: Use libc putchar/getchar instead of wrappers

adc8afb

Fixes warnings about wrong function signature.

tests/shell: Use libc putchar/getchar instead of wrappers

42d6fea

Makefile.buildtests: Indentation fixes

ba9d3f8

Makefile.buildtests: Default to -Werror on make buildtest

0ae1b31

dist/tools/toolchain: Fix whitespace errors in newlib patches.

235b9b3

jnohlgard force-pushed the pr/warning-fixes-3 branch from bd931db to 235b9b3 Compare July 14, 2015 06:12

jnohlgard pushed a commit that referenced this pull request Jul 14, 2015

Merge pull request #3319 from gebart/pr/warning-fixes-3

fa4e059

Makefile.cflags: Build with -Wall -Werror by default (including fixes to correct all existing warnings)

jnohlgard merged commit fa4e059 into RIOT-OS:master Jul 14, 2015

jnohlgard deleted the pr/warning-fixes-3 branch July 14, 2015 06:50

jnohlgard mentioned this pull request Jul 14, 2015

Various: LTO fixes #3361

Closed

4 tasks

thomaseichinger mentioned this pull request Jul 14, 2015

native: fix warning errors freebsd #3390

Merged

thomaseichinger mentioned this pull request Jul 14, 2015

native: fix warning error OS X #3393

Merged

miri64 mentioned this pull request Jul 19, 2015

make: buildtest with "-Wall -Wextra -Werror" #1121

Closed

jnohlgard mentioned this pull request Jul 23, 2015

libcoap: some fixes for debugging enabled #3488

Merged

Conversation

jnohlgard commented Jul 5, 2015

Uh oh!

Kijewski Jul 5, 2015

Choose a reason for hiding this comment

Uh oh!

jnohlgard Jul 6, 2015

Choose a reason for hiding this comment

Uh oh!

jnohlgard commented Jul 6, 2015

Uh oh!

jnohlgard commented Jul 6, 2015

Uh oh!

jnohlgard commented Jul 7, 2015

Uh oh!

OlegHahm commented Jul 7, 2015

Uh oh!

jnohlgard commented Jul 7, 2015

Uh oh!

OlegHahm commented Jul 7, 2015

Uh oh!

Kijewski commented Jul 7, 2015

Uh oh!

jnohlgard commented Jul 7, 2015

Uh oh!

jnohlgard commented Jul 7, 2015

Uh oh!

jnohlgard commented Jul 7, 2015

Uh oh!

jnohlgard commented Jul 7, 2015

Uh oh!

OlegHahm Jul 7, 2015

Choose a reason for hiding this comment

Uh oh!

jnohlgard commented Jul 14, 2015

Uh oh!

OlegHahm commented Jul 14, 2015

Uh oh!

thomaseichinger commented Jul 14, 2015

Uh oh!

jnohlgard commented Jul 14, 2015

Uh oh!

jnohlgard commented Jul 14, 2015

Uh oh!

thomaseichinger commented Jul 14, 2015

Uh oh!

miri64 commented Jul 14, 2015

Uh oh!

thomaseichinger commented Jul 14, 2015

Uh oh!

jnohlgard commented Jul 14, 2015

Uh oh!

thomaseichinger commented Jul 14, 2015

Uh oh!

jnohlgard commented Jul 14, 2015

Uh oh!

thomaseichinger commented Jul 14, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants