`-pedantic-errors` causing problems on old compilers and on some non-Linux systems

[johnsonjh]

Using -pedantic-errors is causing problems on some old compilers on the oldest but still supported Linux distributions, because it implies -pedantic and strict ISO compliance old older but still supported Linux distributions, for example, on Ubuntu 14.04. It's also breaking the build on various other platforms. Sorry I didn't notice this earlier.

Currently, gcc 4.4 with glibc 2.19, on Ubuntu 14.04 LTS (still supported through 2026), running make now fails with:

gcc -O3 -Wall -g -Werror=implicit-function-declaration -pedantic-errors  -c -o obj/codepage.o src/codepage.c
In file included from src/codepage.c:3:
src/env.h:3: error: C++ style comments are not allowed in ISO C90
src/env.h:3: error: (this will be reported only once per input file)
src/codepage.c: In function 'read_codepage_file':
src/codepage.c:213: error: C++ style comments are not allowed in ISO C90
src/codepage.c:213: error: (this will be reported only once per input file)
src/codepage.c:214: error: ISO C90 forbids mixed declarations and code
src/codepage.c: In function 'set_codepage':
src/codepage.c:268: error: 'for' loop initial declarations are only allowed in C99 mode
src/codepage.c:268: note: use option -std=c99 or -std=gnu99 to compile your code
src/codepage.c:271: error: 'for' loop initial declarations are only allowed in C99 mode
src/codepage.c: In function 'list_codepages':
src/codepage.c:289: error: 'for' loop initial declarations are only allowed in C99 mode
src/codepage.c: In function 'get_dos_char':
src/codepage.c:315: error: 'for' loop initial declarations are only allowed in C99 mode

and env CFLAGS="-std=c99 -O3" make now fails with:

gcc -std=c99 -O3 -Wall -g -Werror=implicit-function-declaration -pedantic-errors  -c -o obj/codepage.o src/codepage.c
src/codepage.c: In function 'set_codepage':
src/codepage.c:270: error: implicit declaration of function 'strdup'
src/codepage.c:270: error: initialization makes pointer from integer without a cast
src/codepage.c:273: error: implicit declaration of function 'strcasecmp'

... and because of that flag, even setting -std=gnu99 (or later) can't get around the errors:

src/video.c:24: error: ISO C doesn't support unnamed structs/unions

I can get around this, but it's going to need explicitly setting feature macros (and in a rather complicated way) for it to not cause problems on all the various platforms (Solaris, AIX, etc.) where that flag really changes the compilers behavior by also implying -pedantic and disallowing the use of any extensions.

It is also going to require some source code changes for strict ISO compliance because as noted above you can't use unnamed structs/unions in strict ISO C99 modes.

So, the following patch fixes everything while still using -pedantic-errors, but it's a bit ugly:

```diff diff --git a/GNUmakefile b/GNUmakefile index 290e5b0..ba35ac9 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -46,20 +46,20 @@ uninstall: rm -f $(DESTDIR)${PREFIX}/bin/emu2 # Generated with gcc -MM src/*.c -obj/codepage.o: src/codepage.c src/codepage.h src/dbg.h src/os.h src/env.h +obj/codepage.o: src/codepage.c src/os.h src/codepage.h src/dbg.h src/env.h obj/cpu.o: src/cpu.c src/cpu.h src/dbg.h src/os.h src/dis.h src/emu.h -obj/dbg.o: src/dbg.c src/dbg.h src/os.h src/env.h src/version.h +obj/dbg.o: src/dbg.c src/os.h src/dbg.h src/env.h src/version.h obj/dis.o: src/dis.c src/dis.h src/emu.h -obj/dos.o: src/dos.c src/dos.h src/codepage.h src/dbg.h src/os.h \ - src/dosnames.h src/emu.h src/env.h src/keyb.h src/loader.h \ - src/timer.h src/utils.h src/video.h +obj/dos.o: src/dos.c src/os.h src/dos.h src/codepage.h src/dbg.h \ + src/dosnames.h src/emu.h src/env.h src/keyb.h src/loader.h src/timer.h \ + src/utils.h src/video.h obj/dosnames.o: src/dosnames.c src/dosnames.h src/dbg.h src/os.h src/emu.h \ src/env.h -obj/keyb.o: src/keyb.c src/keyb.h src/codepage.h src/dbg.h src/os.h src/emu.h +obj/keyb.o: src/keyb.c src/os.h src/keyb.h src/codepage.h src/dbg.h src/emu.h obj/loader.o: src/loader.c src/loader.h src/dbg.h src/os.h src/emu.h obj/main.o: src/main.c src/dbg.h src/os.h src/dos.h src/dosnames.h src/emu.h \ src/keyb.h src/timer.h src/video.h -obj/timer.o: src/timer.c src/timer.h src/dbg.h src/os.h src/emu.h -obj/utils.o: src/utils.c src/utils.h src/dbg.h src/os.h -obj/video.o: src/video.c src/video.h src/codepage.h src/dbg.h src/os.h \ +obj/timer.o: src/timer.c src/os.h src/timer.h src/dbg.h src/emu.h +obj/utils.o: src/utils.c src/os.h src/utils.h src/dbg.h +obj/video.o: src/video.c src/os.h src/video.h src/codepage.h src/dbg.h \ src/emu.h src/env.h src/keyb.h diff --git a/platform.mk b/platform.mk index e7513ba..381d53c 100644 --- a/platform.mk +++ b/platform.mk @@ -27,6 +27,23 @@ ifneq "$(findstring clang,$(CC))" "" GCC_CLANG=1 endif +# Detect if GCC (or Clang) needs `-std=c99` +ifeq ($(GCC_CLANG),1) + C99_WR:=$(shell printf '%s\n' \ + "int main(void){for(int i=0;i<1;++i);return 0;}" > .test.c; \ + $(CC) .test.c -o .test.out > /dev/null 2>&1; \ + echo $$?; rm -f .test.c .test.out > /dev/null 2>&1) + ifeq ($(C99_WR),1) + C99_OK:=$(shell printf '%s\n' \ + "int main(void){for(int i=0;i<1;++i);return 0;}" > .test.c; \ + $(CC) -std=c99 .test.c -o .test.out > /dev/null 2>&1; \ + echo $$?; rm -f .test.c .test.out > /dev/null 2>&1) + endif + ifeq ($(C99_OK),0) + CFLAGS+=-std=c99 + endif +endif + # Extra CFLAGS for GCC or Clang EXTRA_CFLAGS?=-Wall -g -Werror=implicit-function-declaration -pedantic-errors ifeq ($(GCC_CLANG),1) diff --git a/src/codepage.c b/src/codepage.c index cb36b52..7df2baf 100644 --- a/src/codepage.c +++ b/src/codepage.c @@ -1,8 +1,26 @@ +#include "os.h" + +#ifndef _XOPEN_SOURCE +# define _XOPEN_SOURCE 600 +#endif + +#if defined(OLD_GLIBC) || !defined(__GLIBC__) +# ifndef _BSD_SOURCE +# define _BSD_SOURCE +# endif +#else +# ifndef _DEFAULT_SOURCE +# define _DEFAULT_SOURCE +# endif +#endif + #include "codepage.h" #include "dbg.h" #include "env.h" + #include #include +#include /* List of code-pages */ struct cp_data diff --git a/src/dbg.c b/src/dbg.c index c53f4f2..0167002 100644 --- a/src/dbg.c +++ b/src/dbg.c @@ -1,7 +1,18 @@ +#include "os.h" + +#if defined(OLD_GLIBC) || !defined(__GLIBC__) +# ifndef _POSIX_SOURCE +# define _POSIX_SOURCE +# endif +#else +# ifndef _DEFAULT_SOURCE +# define _DEFAULT_SOURCE +# endif +#endif + #include "dbg.h" #include "env.h" #include "version.h" -#include "os.h" #include #include diff --git a/src/dis.c b/src/dis.c index 7e693ad..520dec6 100644 --- a/src/dis.c +++ b/src/dis.c @@ -1,4 +1,3 @@ - #include #include #include diff --git a/src/dos.c b/src/dos.c index 52ccdcb..f3d1fe2 100644 --- a/src/dos.c +++ b/src/dos.c @@ -1,3 +1,21 @@ +#include "os.h" + +#if defined(OLD_GLIBC) || !defined(__GLIBC__) +# ifndef _XOPEN_SOURCE +# define _XOPEN_SOURCE 600 +# endif +# ifndef _POSIX_SOURCE +# define _POSIX_SOURCE +# endif +# ifndef _BSD_SOURCE +# define _BSD_SOURCE +# endif +#else +# ifndef _DEFAULT_SOURCE +# define _DEFAULT_SOURCE +# endif +#endif + #include "dos.h" #include "codepage.h" #include "dbg.h" @@ -9,7 +27,6 @@ #include "timer.h" #include "utils.h" #include "video.h" -#include "os.h" #include #include diff --git a/src/dosnames.c b/src/dosnames.c index 2c4eef8..55255b0 100644 --- a/src/dosnames.c +++ b/src/dosnames.c @@ -1,10 +1,12 @@ - -#define _GNU_SOURCE +#ifndef _GNU_SOURCE +# define _GNU_SOURCE +#endif #include "dosnames.h" #include "dbg.h" #include "emu.h" #include "env.h" + #include #include #include diff --git a/src/keyb.c b/src/keyb.c index 04f8fd5..ed51fd4 100644 --- a/src/keyb.c +++ b/src/keyb.c @@ -1,8 +1,37 @@ +#include "os.h" + +#if defined(OLD_GLIBC) || !defined(__GLIBC__) +# if !defined(__FreeBSD__) && !defined(__DragonFly__) && !defined(__NetBSD__) +# ifndef _XOPEN_SOURCE +# define _XOPEN_SOURCE 500 +# endif +# endif +# ifndef _BSD_SOURCE +# define _BSD_SOURCE +# endif +# ifdef __APPLE__ +# ifndef _DARWIN_C_SOURCE +# define _DARWIN_C_SOURCE +# endif +# endif +#else +# ifndef _DEFAULT_SOURCE +# define _DEFAULT_SOURCE +# endif +#endif + +#ifdef _AIX +# define _ALL_SOURCE +#endif + +#ifdef __sun +# define __EXTENSIONS__ +#endif + #include "keyb.h" #include "codepage.h" #include "dbg.h" #include "emu.h" -#include "os.h" #include #include diff --git a/src/main.c b/src/main.c index 7d9c9f5..adc21e0 100644 --- a/src/main.c +++ b/src/main.c @@ -1,5 +1,6 @@ - -#define _GNU_SOURCE +#ifndef _GNU_SOURCE +# define _GNU_SOURCE +#endif #include "dbg.h" #include "dos.h" diff --git a/src/os.h b/src/os.h index 2791aec..ddcf8d4 100644 --- a/src/os.h +++ b/src/os.h @@ -1,6 +1,21 @@ #ifndef OS_H_INCLUDED #define OS_H_INCLUDED +/* Linux feature detection */ +#if defined(__linux__) +# ifndef _GNU_SOURCE +# define _GNU_SOURCE +# endif +# include +#endif + +/* Detect glibc version */ +#if defined(__GLIBC__) +# define GLIBC_VERSION (((0 + __GLIBC__) * 10000) + ((0 + __GLIBC_MINOR__) * 100)) +#endif +#if defined(__GLIBC__) && GLIBC_VERSION > 0 && GLIBC_VERSION <= 21900 +# define OLD_GLIBC +#endif /* Detect proper "does not return" annotation */ #if !defined(NORETURN) diff --git a/src/timer.c b/src/timer.c index dba9b48..056bdfb 100644 --- a/src/timer.c +++ b/src/timer.c @@ -1,3 +1,17 @@ +#include "os.h" + +#if defined(OLD_GCC) || !defined(__GLIBC__) +# ifndef _BSD_SOURCE +# define _BSD_SOURCE +# endif +# ifndef _SVID_SOURCE +# define _SVID_SOURCE +# endif +#else +# ifndef _DEFAULT_SOURCE +# define _DEFAULT_SOURCE +# endif +#endif #include "timer.h" #include "dbg.h" diff --git a/src/utils.c b/src/utils.c index 3d97008..5343dd4 100644 --- a/src/utils.c +++ b/src/utils.c @@ -1,6 +1,20 @@ /* Platform dependent utility functions */ + +#include "os.h" + +#ifdef OLD_GLIBC +# ifndef _XOPEN_SOURCE +# define _XOPEN_SOURCE 500 +# endif +#else +# ifndef _DEFAULT_SOURCE +# define _DEFAULT_SOURCE +# endif +#endif + #include "utils.h" #include "dbg.h" + #include #ifdef __APPLE__ diff --git a/src/video.c b/src/video.c index 47dc0a3..f96454b 100644 --- a/src/video.c +++ b/src/video.c @@ -1,3 +1,23 @@ +#include "os.h" + +#if defined(OLD_GLIBC) || !defined(__GLIBC__) +# ifndef _POSIX_SOURCE +# define _POSIX_SOURCE +# endif +#else +# ifndef _DEFAULT_SOURCE +# define _DEFAULT_SOURCE +# endif +#endif + +#ifdef _AIX +# define _ALL_SOURCE +#endif + +#ifdef __sun +# define __EXTENSIONS__ +#endif + #include "video.h" #include "codepage.h" #include "dbg.h" @@ -21,7 +41,7 @@ union term_cell { uint8_t chr; uint8_t color; - }; + } s; uint16_t value; }; // Simulated character screen. @@ -66,8 +86,8 @@ static void sigwinch_handler(int sig) static union term_cell get_cell(uint8_t chr, uint8_t color) { union term_cell c; - c.chr = chr; - c.color = color; + c.s.chr = chr; + c.s.color = color; return c; } @@ -406,7 +426,7 @@ static void debug_screen(void) union term_cell cell; int16_t vc = vm[x + y * vid_sx]; cell.value = vc; - buf[x] = cell.chr; + buf[x] = cell.s.chr; } buf[vid_sx] = 0; debug(debug_video, "%02u: %s\n", y, buf); @@ -442,7 +462,7 @@ void check_screen(void) union term_cell cell; cell.value = vc; term_screen[y][x] = cell; - put_vc_xy(cell.chr, cell.color, x, y); + put_vc_xy(cell.s.chr, cell.s.color, x, y); } } if(term_cursor != vid_cursor) @@ -555,7 +575,7 @@ static void set_xy_char(unsigned x, unsigned y, uint8_t chr, int page) uint16_t *p = addr_xy(x, y, page); union term_cell cell; cell.value = *p; - cell.chr = chr; + cell.s.chr = chr; *p = cell.value; } @@ -570,7 +590,7 @@ static uint16_t get_xy(unsigned x, unsigned y, int page) uint16_t *vm = (uint16_t *)(memory + 0xB8000 + mem); union term_cell c; c.value = vm[x + y * vid_sx]; - return c.chr | (c.color << 8); + return c.s.chr | (c.s.color << 8); } static void video_putchar(uint8_t ch, uint16_t at, int page) ```

@dmsc I could clean that up a little bit more but I don’t like it.

So, I'll propose that you just remove the -pedantic-errors flag, for simplicity.

That way env CFLAGS="-std=gnu99 -O3" make would be sufficent for building on such a configuration, without any of the above patching being needed.

[johnsonjh]

I'll probably do the "Detect if GCC (or Clang) needs -std=c99" addition in another PR though, that way those really old gcc versions build fine with just make.

Edit: PR #111

[dmsc]

Hi!

Yes, it is best to simply remove the flag. I do like compiling with pedantic flags, to detect possible errors.

[johnsonjh]

@dmsc

Check out direnv - we use this on other projects I work on.

You can make a “.envrc” file which contains export CFLAGS="-whatever" or other variables and it gets set automatically, and when you are working in that directory only, so you can have much stricter flags when developing.

(I did have to change my aliases for git clean-ing to do git clean -fdx -e .envrc so as to not blow away the file, if adding it to the .gitignore)

[dmsc]

Did not knew direnv. Here what we use is to include a local.mk file from the main makefile, this also allows to include custom variables. By using -include local.mk, make ignores the error if the file does not exists.