From b28a16c0da3917260f1d2dedc33cb80b27f30eaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Sun, 1 Apr 2018 00:47:35 +0100 Subject: [PATCH 01/12] Enable -Wpedantic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a few warnings related needed to build the source with this option. Add new exception to .checkpatch.conf. Signed-off-by: Antonio Niño Díaz --- .checkpatch.conf | 3 +++ Makefile | 2 +- src/asm/lexer.c | 1 - src/asm/main.c | 5 ++++- src/gfx/makepng.c | 4 ++-- 5 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.checkpatch.conf b/.checkpatch.conf index 71f34a2f..4f4ed543 100644 --- a/.checkpatch.conf +++ b/.checkpatch.conf @@ -53,6 +53,9 @@ # Prefer stdint.h types over kernel types --ignore PREFER_KERNEL_TYPES +# Don't ask to replace sscanf by kstrto +--ignore SSCANF_TO_KSTRTO + # Parentheses can make the code clearer --ignore UNNECESSARY_PARENTHESES diff --git a/Makefile b/Makefile index 49da9f93..125f1acf 100644 --- a/Makefile +++ b/Makefile @@ -26,7 +26,7 @@ PNGLDLIBS := `${PKG_CONFIG} --static --libs-only-l libpng` VERSION_STRING := `git describe --tags --dirty --always 2>/dev/null` -WARNFLAGS := -Wall -Werror +WARNFLAGS := -Werror -Wall -Wpedantic # Overridable CFLAGS CFLAGS := -g diff --git a/src/asm/lexer.c b/src/asm/lexer.c index 8696b140..b789f567 100644 --- a/src/asm/lexer.c +++ b/src/asm/lexer.c @@ -640,7 +640,6 @@ scanagain: /* Check for line continuation character */ if (*pLexBuffer == '\\') { - /* * Look for line continuation character after a series of * spaces. This is also useful for files that use Windows line diff --git a/src/asm/main.c b/src/asm/main.c index 28659e53..4a551a88 100644 --- a/src/asm/main.c +++ b/src/asm/main.c @@ -147,10 +147,13 @@ void opt_Parse(char *s) case 'z': if (strlen(&s[1]) <= 2) { int32_t result; + unsigned int fillchar; - result = sscanf(&s[1], "%x", &newopt.fillchar); + result = sscanf(&s[1], "%x", &fillchar); if (!((result == EOF) || (result == 1))) errx(1, "Invalid argument for option 'z'"); + + newopt.fillchar = fillchar; } else { errx(1, "Invalid argument for option 'z'"); } diff --git a/src/gfx/makepng.c b/src/gfx/makepng.c index 3f8fc24a..88654afb 100644 --- a/src/gfx/makepng.c +++ b/src/gfx/makepng.c @@ -317,9 +317,9 @@ static void rgba_png_palette(struct PNGImage *img, png_color **palette_ptr_ptr, int *num_colors) { if (png_get_valid(img->png, img->info, PNG_INFO_PLTE)) - return rgba_PLTE_palette(img, palette_ptr_ptr, num_colors); + rgba_PLTE_palette(img, palette_ptr_ptr, num_colors); else - return rgba_build_palette(img, palette_ptr_ptr, num_colors); + rgba_build_palette(img, palette_ptr_ptr, num_colors); } static void rgba_PLTE_palette(struct PNGImage *img, From 9829be104549bcdef63984676bf9c432bbb777fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Sun, 1 Apr 2018 01:18:29 +0100 Subject: [PATCH 02/12] Enable -Wextra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit -Wsign-compare has been disabled because flex generates a comparison that triggers a warning and cannot be fixed in the code. Signed-off-by: Antonio Niño Díaz --- Makefile | 2 +- src/asm/asmy.y | 3 ++- src/asm/fstack.c | 6 ++++-- src/asm/globlex.c | 2 +- src/asm/main.c | 2 +- src/asm/symbol.c | 4 ++-- src/gfx/main.c | 1 + 7 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 125f1acf..0964a45e 100644 --- a/Makefile +++ b/Makefile @@ -26,7 +26,7 @@ PNGLDLIBS := `${PKG_CONFIG} --static --libs-only-l libpng` VERSION_STRING := `git describe --tags --dirty --always 2>/dev/null` -WARNFLAGS := -Werror -Wall -Wpedantic +WARNFLAGS := -Werror -Wall -Wextra -Wpedantic -Wno-sign-compare # Overridable CFLAGS CFLAGS := -g diff --git a/src/asm/asmy.y b/src/asm/asmy.y index ea964260..93d3f0f4 100644 --- a/src/asm/asmy.y +++ b/src/asm/asmy.y @@ -341,8 +341,9 @@ static void if_skip_to_else(void) break; case '\"': - src++; + src += 2; inString = false; + break; default: src++; diff --git a/src/asm/fstack.c b/src/asm/fstack.c index 6b7141e9..19d58e20 100644 --- a/src/asm/fstack.c +++ b/src/asm/fstack.c @@ -257,8 +257,10 @@ FILE *fstk_FindFile(char *fname) * space had been available. Thus, a return value of `size` or * more means that the output was truncated. */ - if (snprintf(path, sizeof(path), "%s%s", IncludePaths[i], fname) - >= sizeof(path)) + int fullpathlen = snprintf(path, sizeof(path), "%s%s", + IncludePaths[i], fname); + + if (fullpathlen >= (int)sizeof(path)) continue; f = fopen(path, "rb"); diff --git a/src/asm/globlex.c b/src/asm/globlex.c index 79a8583d..9dae47bc 100644 --- a/src/asm/globlex.c +++ b/src/asm/globlex.c @@ -216,7 +216,7 @@ uint32_t PutMacroArg(char *src, uint32_t size) return 0; } -uint32_t PutUniqueArg(char *src, uint32_t size) +uint32_t PutUniqueArg(__attribute ((unused)) char *src, uint32_t size) { char *s; diff --git a/src/asm/main.c b/src/asm/main.c index 4a551a88..3996ff89 100644 --- a/src/asm/main.c +++ b/src/asm/main.c @@ -225,7 +225,7 @@ void opt_AddDefine(char *s) static void opt_ParseDefines(void) { - int32_t i; + uint32_t i; for (i = 0; i < cldefines_index; i += 2) sym_AddString(cldefines[i], cldefines[i + 1]); diff --git a/src/asm/symbol.c b/src/asm/symbol.c index c673e10c..24bcb385 100644 --- a/src/asm/symbol.c +++ b/src/asm/symbol.c @@ -62,7 +62,7 @@ void helper_RemoveLeadingZeros(char *string) memmove(string, new_beginning, strlen(new_beginning) + 1); } -int32_t Callback_NARG(struct sSymbol *sym) +int32_t Callback_NARG(__attribute__ ((unused)) struct sSymbol *sym) { uint32_t i = 0; @@ -673,7 +673,7 @@ void sym_AddReloc(char *tzSym) struct sSymbol *parent = pScope->pScope ? pScope->pScope : pScope; - int32_t parentLen = localPtr - tzSym; + uint32_t parentLen = localPtr - tzSym; if (strchr(localPtr + 1, '.') != NULL) { fatalerror("'%s' is a nonsensical reference to a nested local symbol", diff --git a/src/gfx/main.c b/src/gfx/main.c index 6171676d..b58e39f8 100644 --- a/src/gfx/main.c +++ b/src/gfx/main.c @@ -53,6 +53,7 @@ int main(int argc, char *argv[]) break; case 'F': opts.hardfix = true; + /* fallthrough */ case 'f': opts.fix = true; break; From 516e4578ea144e297e69ac7972e639b67a17b387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Sun, 1 Apr 2018 01:38:07 +0100 Subject: [PATCH 03/12] Enable more optional warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit -Wformat-truncation is set to 1 instead of 2 because in some cases the return value of snprintf is used to warn about truncations but GCC still creates a warning for it, preventing the compilation from continuing. Signed-off-by: Antonio Niño Díaz --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 0964a45e..75f18e60 100644 --- a/Makefile +++ b/Makefile @@ -26,7 +26,9 @@ PNGLDLIBS := `${PKG_CONFIG} --static --libs-only-l libpng` VERSION_STRING := `git describe --tags --dirty --always 2>/dev/null` -WARNFLAGS := -Werror -Wall -Wextra -Wpedantic -Wno-sign-compare +WARNFLAGS := -Werror -Wall -Wextra -Wpedantic -Wno-sign-compare -Wchkp \ + -Wformat=2 -Wformat-overflow=2 -Wformat-truncation=1 \ + -Wformat-y2k # Overridable CFLAGS CFLAGS := -g From 85ece882689527ad8f73cf05637dfca2b3fcb048 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Sun, 1 Apr 2018 01:54:42 +0100 Subject: [PATCH 04/12] Add default clauses to switch statements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Antonio Niño Díaz --- src/asm/fstack.c | 6 ++++++ src/asm/globlex.c | 3 +++ src/asm/lexer.c | 4 ++-- src/link/assign.c | 3 +++ src/link/patch.c | 2 ++ 5 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/asm/fstack.c b/src/asm/fstack.c index 19d58e20..ead50395 100644 --- a/src/asm/fstack.c +++ b/src/asm/fstack.c @@ -87,6 +87,8 @@ static void pushcontext(void) (*ppFileStack)->nREPTBlockSize = nCurrentREPTBlockSize; (*ppFileStack)->nREPTBlockCount = nCurrentREPTBlockCount; break; + default: + fatalerror("%s: Internal error.", __func__); } nLineNo = 0; @@ -152,6 +154,8 @@ static int32_t popcontext(void) nCurrentREPTBlockSize = pLastFile->nREPTBlockSize; nCurrentREPTBlockCount = pLastFile->nREPTBlockCount; break; + default: + fatalerror("%s: Internal error.", __func__); } free(*ppLastFile); @@ -174,6 +178,8 @@ int32_t fstk_GetLine(void) return nLineNo; /* ??? */ case STAT_isREPTBlock: break; /* Peek top file of the stack */ + default: + fatalerror("%s: Internal error.", __func__); } pLastFile = pFileStack; diff --git a/src/asm/globlex.c b/src/asm/globlex.c index 9dae47bc..b25afde0 100644 --- a/src/asm/globlex.c +++ b/src/asm/globlex.c @@ -93,6 +93,9 @@ static int32_t ascii2bin(char *s) s += 1; convertfunc = binary2bin; break; + default: + /* Handle below */ + break; } if (radix == 4) { diff --git a/src/asm/lexer.c b/src/asm/lexer.c index b789f567..fd0b3d17 100644 --- a/src/asm/lexer.c +++ b/src/asm/lexer.c @@ -850,7 +850,7 @@ uint32_t yylex(void) return yylex_NORMAL(); case LEX_STATE_MACROARGS: return yylex_MACROARGS(); + default: + fatalerror("%s: Internal error.", __func__); } - - fatalerror("Internal error in %s", __func__); } diff --git a/src/link/assign.c b/src/link/assign.c index fca668cd..73c644ae 100644 --- a/src/link/assign.c +++ b/src/link/assign.c @@ -617,6 +617,9 @@ void AssignSections(void) pSection->nOrg, pSection->nBank); } break; + default: + errx(1, "%s: Internal error: Type %d", __func__, + pSection->Type); } } diff --git a/src/link/patch.c b/src/link/patch.c index 9d704264..f3d036bf 100644 --- a/src/link/patch.c +++ b/src/link/patch.c @@ -325,6 +325,8 @@ void Patch(void) pPatch->nLineNo); } break; + default: + errx(1, "%s: Internal error.", __func__); } pPatch = pPatch->pNext; From 340362d98468a8a52b4ddc5dd19a6281b79fa8f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Sun, 1 Apr 2018 02:09:20 +0100 Subject: [PATCH 05/12] Enable a few warning flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Antonio Niño Díaz --- Makefile | 7 ++++++- src/asm/lexer.c | 2 +- src/asm/main.c | 3 ++- src/gfx/makepng.c | 6 ++++-- src/link/assign.c | 11 +++++++++-- src/link/lexer.l | 2 +- src/link/parser.y | 6 +++--- src/link/script.c | 46 +++++++++++++++++++++++----------------------- 8 files changed, 49 insertions(+), 34 deletions(-) diff --git a/Makefile b/Makefile index 75f18e60..433b5988 100644 --- a/Makefile +++ b/Makefile @@ -28,7 +28,12 @@ VERSION_STRING := `git describe --tags --dirty --always 2>/dev/null` WARNFLAGS := -Werror -Wall -Wextra -Wpedantic -Wno-sign-compare -Wchkp \ -Wformat=2 -Wformat-overflow=2 -Wformat-truncation=1 \ - -Wformat-y2k + -Wformat-y2k -Wswitch-enum -Wunused -Wuninitialized \ + -Wunknown-pragmas -Wstrict-overflow=5 -Wstringop-overflow=4 \ + -Walloc-zero -Wduplicated-cond -Wfloat-equal -Wshadow \ + -Wcast-qual -Wcast-align -Wlogical-op -Wnested-externs \ + -Wno-aggressive-loop-optimizations -Winline \ + -Wstrict-prototypes -Wold-style-definition # Overridable CFLAGS CFLAGS := -g diff --git a/src/asm/lexer.c b/src/asm/lexer.c index fd0b3d17..20b6690a 100644 --- a/src/asm/lexer.c +++ b/src/asm/lexer.c @@ -414,7 +414,7 @@ void yylex_GetFloatMaskAndFloatLen(uint32_t *pnFloatMask, uint32_t *pnFloatLen) /* * Gets the longest keyword/operator from the current position in the buffer. */ -struct sLexString *yylex_GetLongestFixed() +struct sLexString *yylex_GetLongestFixed(void) { struct sLexString *pLongestFixed = NULL; char *s = pLexBuffer; diff --git a/src/asm/main.c b/src/asm/main.c index 3996ff89..4be82181 100644 --- a/src/asm/main.c +++ b/src/asm/main.c @@ -6,6 +6,7 @@ * SPDX-License-Identifier: MIT */ +#include #include #include #include @@ -483,7 +484,7 @@ int main(int argc, char *argv[]) if (CurrentOptions.verbose) { printf("Success! %u lines in %d.%02d seconds ", nTotalLines, (int)timespent, ((int)(timespent * 100.0)) % 100); - if (timespent == 0) + if (timespent < FLT_MIN_EXP) printf("(INFINITY lines/minute)\n"); else printf("(%d lines/minute)\n", diff --git a/src/gfx/makepng.c b/src/gfx/makepng.c index 88654afb..38f54d4c 100644 --- a/src/gfx/makepng.c +++ b/src/gfx/makepng.c @@ -469,8 +469,10 @@ struct ColorWithLuminance { static int compare_luminance(const void *a, const void *b) { - struct ColorWithLuminance *x = (struct ColorWithLuminance *)a; - struct ColorWithLuminance *y = (struct ColorWithLuminance *)b; + const struct ColorWithLuminance *x, *y; + + x = (const struct ColorWithLuminance *)a; + y = (const struct ColorWithLuminance *)b; return y->luminance - x->luminance; } diff --git a/src/link/assign.c b/src/link/assign.c index 73c644ae..e2c4b1ee 100644 --- a/src/link/assign.c +++ b/src/link/assign.c @@ -74,6 +74,10 @@ static void do_max_bank(enum eSectionType Type, int32_t nBank) if (nBank > MaxVBankUsed) MaxVBankUsed = nBank; break; + case SECT_ROM0: + case SECT_WRAM0: + case SECT_OAM: + case SECT_HRAM: default: break; } @@ -494,7 +498,6 @@ void SetLinkerscriptName(char *tzLinkerscriptFile) void AssignSections(void) { - int32_t i; struct sSection *pSection; MaxBankUsed = 0; @@ -503,7 +506,7 @@ void AssignSections(void) * Initialize the memory areas */ - for (i = 0; i < BANK_INDEX_MAX; i += 1) { + for (int32_t i = 0; i < BANK_INDEX_MAX; i += 1) { BankFree[i] = malloc(sizeof(*BankFree[i])); if (!BankFree[i]) { @@ -663,6 +666,10 @@ void AssignSections(void) do_max_bank(pSection->Type, pSection->nBank); break; + case SECT_ROM0: + case SECT_WRAM0: + case SECT_OAM: + case SECT_HRAM: default: /* Handle other sections later */ break; } diff --git a/src/link/lexer.l b/src/link/lexer.l index 156ddf0c..ac5a4fff 100644 --- a/src/link/lexer.l +++ b/src/link/lexer.l @@ -23,7 +23,7 @@ #include "types.h" -extern int yyparse(); +extern int yyparse(void); /* File include stack. */ diff --git a/src/link/parser.y b/src/link/parser.y index a532f560..8e6a2094 100644 --- a/src/link/parser.y +++ b/src/link/parser.y @@ -14,7 +14,7 @@ #include "link/script.h" -int yylex(); +int yylex(void); void yyerror(char *); extern int yylineno; @@ -107,8 +107,8 @@ statement: %% -extern int yylex(); -extern int yyparse(); +extern int yylex(void); +extern int yyparse(void); int yywrap(void) { diff --git a/src/link/script.c b/src/link/script.c index 55e6c193..913b69ad 100644 --- a/src/link/script.c +++ b/src/link/script.c @@ -85,59 +85,59 @@ void script_InitSections(void) } } -void script_SetCurrentSectionType(const char *type, uint32_t bank) +void script_SetCurrentSectionType(const char *type, uint32_t bank_num) { if (strcmp(type, "ROM0") == 0) { - if (bank != 0) + if (bank_num != 0) errx(1, "Trying to assign a bank number to ROM0.\n"); current_bank = BANK_INDEX_ROM0; current_real_bank = 0; return; } else if (strcmp(type, "ROMX") == 0) { - if (bank == 0) + if (bank_num == 0) errx(1, "ROMX index can't be 0.\n"); - if (bank > BANK_COUNT_ROMX) { - errx(1, "ROMX index too big (%d > %d).\n", bank, + if (bank_num > BANK_COUNT_ROMX) { + errx(1, "ROMX index too big (%d > %d).\n", bank_num, BANK_COUNT_ROMX); } - current_bank = BANK_INDEX_ROMX + bank - 1; - current_real_bank = bank; + current_bank = BANK_INDEX_ROMX + bank_num - 1; + current_real_bank = bank_num; return; } else if (strcmp(type, "VRAM") == 0) { - if (bank >= BANK_COUNT_VRAM) { - errx(1, "VRAM index too big (%d >= %d).\n", bank, + if (bank_num >= BANK_COUNT_VRAM) { + errx(1, "VRAM index too big (%d >= %d).\n", bank_num, BANK_COUNT_VRAM); } - current_bank = BANK_INDEX_VRAM + bank; - current_real_bank = bank; + current_bank = BANK_INDEX_VRAM + bank_num; + current_real_bank = bank_num; return; } else if (strcmp(type, "WRAM0") == 0) { - if (bank != 0) + if (bank_num != 0) errx(1, "Trying to assign a bank number to WRAM0.\n"); current_bank = BANK_INDEX_WRAM0; current_real_bank = 0; return; } else if (strcmp(type, "WRAMX") == 0) { - if (bank == 0) + if (bank_num == 0) errx(1, "WRAMX index can't be 0.\n"); - if (bank > BANK_COUNT_WRAMX) { - errx(1, "WRAMX index too big (%d > %d).\n", bank, + if (bank_num > BANK_COUNT_WRAMX) { + errx(1, "WRAMX index too big (%d > %d).\n", bank_num, BANK_COUNT_WRAMX); } - current_bank = BANK_INDEX_WRAMX + bank - 1; - current_real_bank = bank; + current_bank = BANK_INDEX_WRAMX + bank_num - 1; + current_real_bank = bank_num; return; } else if (strcmp(type, "SRAM") == 0) { - if (bank >= BANK_COUNT_SRAM) { - errx(1, "SRAM index too big (%d >= %d).\n", bank, + if (bank_num >= BANK_COUNT_SRAM) { + errx(1, "SRAM index too big (%d >= %d).\n", bank_num, BANK_COUNT_SRAM); } - current_bank = BANK_INDEX_SRAM + bank; - current_real_bank = bank; + current_bank = BANK_INDEX_SRAM + bank_num; + current_real_bank = bank_num; return; } else if (strcmp(type, "OAM") == 0) { - if (bank != 0) { + if (bank_num != 0) { errx(1, "%s: Trying to assign a bank number to OAM.\n", __func__); } @@ -145,7 +145,7 @@ void script_SetCurrentSectionType(const char *type, uint32_t bank) current_real_bank = 0; return; } else if (strcmp(type, "HRAM") == 0) { - if (bank != 0) { + if (bank_num != 0) { errx(1, "%s: Trying to assign a bank number to HRAM.\n", __func__); } From 29253046d59b7ae944f8d9c9164e27228852b4cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Mon, 2 Apr 2018 00:19:33 +0100 Subject: [PATCH 06/12] Remove C++ check in stdnoreturn.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This isn't a C++ project, only keep checks for C compilers. Signed-off-by: Antonio Niño Díaz --- include/extern/stdnoreturn.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/extern/stdnoreturn.h b/include/extern/stdnoreturn.h index c6df9928..133f9c88 100644 --- a/include/extern/stdnoreturn.h +++ b/include/extern/stdnoreturn.h @@ -12,9 +12,6 @@ #if __STDC_VERSION__ >= 201112L /* C11 or newer */ #define noreturn _Noreturn -#elif __cplusplus >= 201103L - /* C++11 or newer */ - #define noreturn [[noreturn]] #elif __GNUC__ > 2 || (__GNUC__ == 2 && (__GNUC_MINOR__ >= 5)) /* GCC 2.5 or newer */ #define noreturn __attribute__ ((noreturn)) From 95ccc48d0c30380bfc4367f442f3fea1af75e79a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Mon, 2 Apr 2018 00:25:26 +0100 Subject: [PATCH 07/12] Enable -Wundef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Antonio Niño Díaz --- Makefile | 2 +- include/extern/stdnoreturn.h | 32 ++++++++++++++++++++++---------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 433b5988..35b2625e 100644 --- a/Makefile +++ b/Makefile @@ -32,7 +32,7 @@ WARNFLAGS := -Werror -Wall -Wextra -Wpedantic -Wno-sign-compare -Wchkp \ -Wunknown-pragmas -Wstrict-overflow=5 -Wstringop-overflow=4 \ -Walloc-zero -Wduplicated-cond -Wfloat-equal -Wshadow \ -Wcast-qual -Wcast-align -Wlogical-op -Wnested-externs \ - -Wno-aggressive-loop-optimizations -Winline \ + -Wno-aggressive-loop-optimizations -Winline -Wundef \ -Wstrict-prototypes -Wold-style-definition # Overridable CFLAGS diff --git a/include/extern/stdnoreturn.h b/include/extern/stdnoreturn.h index 133f9c88..bd004267 100644 --- a/include/extern/stdnoreturn.h +++ b/include/extern/stdnoreturn.h @@ -9,16 +9,28 @@ #ifndef EXTERN_STDNORETURN_H #define EXTERN_STDNORETURN_H -#if __STDC_VERSION__ >= 201112L - /* C11 or newer */ - #define noreturn _Noreturn -#elif __GNUC__ > 2 || (__GNUC__ == 2 && (__GNUC_MINOR__ >= 5)) - /* GCC 2.5 or newer */ - #define noreturn __attribute__ ((noreturn)) -#elif _MSC_VER >= 1310 - /* MS Visual Studio 2003/.NET Framework 1.1 or newer */ - #define noreturn _declspec(noreturn) -#else +#if defined(__STDC_VERSION__) + #if __STDC_VERSION__ >= 201112L + /* C11 or newer */ + #define noreturn _Noreturn + #endif +#endif + +#if defined(__GNUC__) && !defined(noreturn) + #if __GNUC__ > 2 || (__GNUC__ == 2 && (__GNUC_MINOR__ >= 5)) + /* GCC 2.5 or newer */ + #define noreturn __attribute__ ((noreturn)) + #endif +#endif + +#if defined(_MSC_VER) && !defined(noreturn) + #if _MSC_VER >= 1310 + /* MS Visual Studio 2003/.NET Framework 1.1 or newer */ + #define noreturn _declspec(noreturn) + #endif +#endif + +#if !defined(noreturn) /* Unsupported, but no need to throw a fit */ #define noreturn #endif From 0ae69b3114b096cf1b0a1ed6a5740db0b00b0955 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Mon, 2 Apr 2018 00:39:12 +0100 Subject: [PATCH 08/12] Rename stdnoreturn.h to helpers.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This file will contain more compiler-specific helpers. Signed-off-by: Antonio Niño Díaz --- include/asm/main.h | 2 +- include/extern/err.h | 2 +- include/{extern/stdnoreturn.h => helpers.h} | 6 +++--- include/link/script.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) rename include/{extern/stdnoreturn.h => helpers.h} (88%) diff --git a/include/asm/main.h b/include/asm/main.h index 7eed1df4..77f3e854 100644 --- a/include/asm/main.h +++ b/include/asm/main.h @@ -12,7 +12,7 @@ #include #include -#include "extern/stdnoreturn.h" +#include "helpers.h" struct sOptions { char binary[2]; diff --git a/include/extern/err.h b/include/extern/err.h index aa272a0a..291e20a5 100644 --- a/include/extern/err.h +++ b/include/extern/err.h @@ -17,7 +17,7 @@ #include -#include "extern/stdnoreturn.h" +#include "helpers.h" #define warn rgbds_warn #define vwarn rgbds_vwarn diff --git a/include/extern/stdnoreturn.h b/include/helpers.h similarity index 88% rename from include/extern/stdnoreturn.h rename to include/helpers.h index bd004267..05b1c710 100644 --- a/include/extern/stdnoreturn.h +++ b/include/helpers.h @@ -6,8 +6,8 @@ * SPDX-License-Identifier: MIT */ -#ifndef EXTERN_STDNORETURN_H -#define EXTERN_STDNORETURN_H +#ifndef HELPERS_H +#define HELPERS_H #if defined(__STDC_VERSION__) #if __STDC_VERSION__ >= 201112L @@ -35,4 +35,4 @@ #define noreturn #endif -#endif /* EXTERN_STDNORETURN_H */ +#endif /* HELPERS_H */ diff --git a/include/link/script.h b/include/link/script.h index 8796d3c8..14e9b2f8 100644 --- a/include/link/script.h +++ b/include/link/script.h @@ -11,7 +11,7 @@ #include -#include "extern/stdnoreturn.h" +#include "helpers.h" noreturn void script_fatalerror(const char *fmt, ...); From e99a651165c104daae1dba4737e23f964a181ee8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Mon, 2 Apr 2018 22:07:10 +0100 Subject: [PATCH 09/12] Create makefile target to check all warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove '-Werror' from the default make target to make it easy for regular users of RGBDS to compile the source code. Only a few basic warnings are left in that target. All the warnings have been moved to a new target called 'develop'. This target is now the one used in Travis CI to check for problems during compilation. Signed-off-by: Antonio Niño Díaz --- CONTRIBUTING.rst | 19 ++++++++++++------- Makefile | 29 +++++++++++++++++++---------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 209967df..64ee0661 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -70,24 +70,29 @@ copyright and the reference to the MIT License. 3. Create a new branch to work on. You could still work on ``develop``, but it's easier that way. -4. Sign off your commits: ``git commit -s`` +4. Compile your changes with ``make develop`` instead of just ``make``. This + target checks for additional warnings. Your patches shouldn't introduce any + new warning (but it may be possible to remove some warning checks if it makes + the code much easier). -5. Follow the Linux kernel coding style, which can be found in the file +5. Sign off your commits: ``git commit -s`` + +6. Follow the Linux kernel coding style, which can be found in the file ``Documentation/process/coding-style.rst`` in the Linux kernel repository. Note that the coding style isn't writen on stone, if there is a good reason to deviate from it, it should be fine. -6. Download the files ``checkpatch.pl``, ``const_structs.checkpatch`` and +7. Download the files ``checkpatch.pl``, ``const_structs.checkpatch`` and ``spelling.txt`` from the folder ``scripts`` in the Linux kernel repository. -7. To use ``checkpatch.pl`` you can use ``make checkpatch``, which will check +8. To use ``checkpatch.pl`` you can use ``make checkpatch``, which will check the coding style of all patches between the current one and the upstream code. By default, the Makefile expects the script (and associate files) to be located in ``../linux/scripts/``, but you can place them anywhere you like as long as you specify it when executing the command: ``CHECKPATCH=../path/to/folder make checkpatch``. -8. Create a pull request against the branch ``develop``. +9. Create a pull request against the branch ``develop``. -9. Be prepared to get some comments about your code and to modify it. Tip: Use - ``git rebase -i origin/develop`` to modify chains of commits. +10. Be prepared to get some comments about your code and to modify it. Tip: Use + ``git rebase -i origin/develop`` to modify chains of commits. diff --git a/Makefile b/Makefile index 35b2625e..58a51b6e 100644 --- a/Makefile +++ b/Makefile @@ -26,14 +26,7 @@ PNGLDLIBS := `${PKG_CONFIG} --static --libs-only-l libpng` VERSION_STRING := `git describe --tags --dirty --always 2>/dev/null` -WARNFLAGS := -Werror -Wall -Wextra -Wpedantic -Wno-sign-compare -Wchkp \ - -Wformat=2 -Wformat-overflow=2 -Wformat-truncation=1 \ - -Wformat-y2k -Wswitch-enum -Wunused -Wuninitialized \ - -Wunknown-pragmas -Wstrict-overflow=5 -Wstringop-overflow=4 \ - -Walloc-zero -Wduplicated-cond -Wfloat-equal -Wshadow \ - -Wcast-qual -Wcast-align -Wlogical-op -Wnested-externs \ - -Wno-aggressive-loop-optimizations -Winline -Wundef \ - -Wstrict-prototypes -Wold-style-definition +WARNFLAGS := -Wall # Overridable CFLAGS CFLAGS := -g @@ -167,6 +160,7 @@ install: all # Target used to check the coding style of the whole codebase. '.y' and '.l' # files aren't checked, unfortunately... + checkcodebase: $Qfor file in `git ls-files | grep -E '\.c|\.h' | grep -v '\.html'`; do \ ${CHECKPATCH} -f "$$file"; \ @@ -176,6 +170,7 @@ checkcodebase: # to the HEAD. Runs checkpatch once for each commit between the current HEAD and # the first common commit between the HEAD and origin/develop. '.y' and '.l' # files aren't checked, unfortunately... + checkpatch: $Qeval COMMON_COMMIT=$$(git merge-base HEAD origin/develop); \ for commit in `git rev-list $$COMMON_COMMIT..HEAD`; do \ @@ -200,6 +195,20 @@ wwwman: $Qmandoc ${MANDOC} src/link/rgblink.5 > docs/rgblink.5.html $Qmandoc ${MANDOC} src/gfx/rgbgfx.1 > docs/rgbgfx.1.html +# This target is used during development in order to prevent adding new issues +# to the source code. All warnings are treated as errors in order to block the +# compilation and make the continous integration infrastructure return failure. + +develop: + $Qenv make -j WARNFLAGS="-Werror -Wall -Wextra -Wpedantic \ + -Wno-sign-compare -Wchkp -Wformat=2 -Wformat-overflow=2 \ + -Wformat-truncation=1 -Wformat-y2k -Wswitch-enum -Wunused \ + -Wuninitialized -Wunknown-pragmas -Wstrict-overflow=5 \ + -Wstringop-overflow=4 -Walloc-zero -Wduplicated-cond \ + -Wfloat-equal -Wshadow -Wcast-qual -Wcast-align -Wlogical-op \ + -Wnested-externs -Wno-aggressive-loop-optimizations -Winline \ + -Wundef -Wstrict-prototypes -Wold-style-definition" + # Targets for the project maintainer to easily create Windows exes. # This is not for Windows users! # If you're building on Windows with Cygwin or Mingw, just follow the Unix @@ -207,7 +216,7 @@ wwwman: mingw32: $Qenv PKG_CONFIG_PATH=/usr/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig/ \ - make CC=i686-w64-mingw32-gcc YACC=bison WARNFLAGS= -j + make CC=i686-w64-mingw32-gcc YACC=bison -j $Qmv rgbasm rgbasm.exe $Qmv rgblink rgblink.exe $Qmv rgbfix rgbfix.exe @@ -215,7 +224,7 @@ mingw32: mingw64: $Qenv PKG_CONFIG_PATH=/usr/x86_64-w64-mingw32/sys-root/mingw/lib/pkgconfig/ \ - make CC=x86_64-w64-mingw32-gcc YACC=bison WARNFLAGS= -j + make CC=x86_64-w64-mingw32-gcc YACC=bison -j $Qmv rgbasm rgbasm.exe $Qmv rgblink rgblink.exe $Qmv rgbfix rgbfix.exe From 895d1d581382bfbcf67827318328d6efc1be463f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Mon, 2 Apr 2018 22:12:15 +0100 Subject: [PATCH 10/12] Remove check for C11 in helpers.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mandatory CFLAGS in the Makefile make the compiler use C99. Signed-off-by: Antonio Niño Díaz --- include/helpers.h | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/include/helpers.h b/include/helpers.h index 05b1c710..0f471a08 100644 --- a/include/helpers.h +++ b/include/helpers.h @@ -9,14 +9,7 @@ #ifndef HELPERS_H #define HELPERS_H -#if defined(__STDC_VERSION__) - #if __STDC_VERSION__ >= 201112L - /* C11 or newer */ - #define noreturn _Noreturn - #endif -#endif - -#if defined(__GNUC__) && !defined(noreturn) +#if defined(__GNUC__) #if __GNUC__ > 2 || (__GNUC__ == 2 && (__GNUC_MINOR__ >= 5)) /* GCC 2.5 or newer */ #define noreturn __attribute__ ((noreturn)) From cbaaec98ca14ecb6bc8521b9fb7a9c319cb20f4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Mon, 2 Apr 2018 22:14:24 +0100 Subject: [PATCH 11/12] Simplify helpers.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `__attribute__((noreturn))` has been supported since GCC 2.5, that was released October 22, 1993. It doesn't make sense to check if the version is at least that one, we are compiling for C99, that is more modern. [1] Also, remove the MSVC check. This code is never compiled with it so there may be problems that need to be solved to make it compile. All releases cross-compiled from linux. If there is an actual need to support MSVC, the compiler definitions can be added again. Also, if the compiler is not supported, the compiler helpers default to nothing, so the code can still compile. [1] https://gcc.gnu.org/onlinedocs/ Signed-off-by: Antonio Niño Díaz --- include/helpers.h | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/include/helpers.h b/include/helpers.h index 0f471a08..3a3cc476 100644 --- a/include/helpers.h +++ b/include/helpers.h @@ -9,21 +9,10 @@ #ifndef HELPERS_H #define HELPERS_H -#if defined(__GNUC__) - #if __GNUC__ > 2 || (__GNUC__ == 2 && (__GNUC_MINOR__ >= 5)) - /* GCC 2.5 or newer */ - #define noreturn __attribute__ ((noreturn)) - #endif -#endif - -#if defined(_MSC_VER) && !defined(noreturn) - #if _MSC_VER >= 1310 - /* MS Visual Studio 2003/.NET Framework 1.1 or newer */ - #define noreturn _declspec(noreturn) - #endif -#endif - -#if !defined(noreturn) +#ifdef __GNUC__ + /* GCC or compatible */ + #define noreturn __attribute__ ((noreturn)) +#else /* Unsupported, but no need to throw a fit */ #define noreturn #endif From 1b4187e51f76dea56924b610c98840f0828dd9e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Ni=C3=B1o=20D=C3=ADaz?= Date: Mon, 2 Apr 2018 22:25:09 +0100 Subject: [PATCH 12/12] Cleanup GCC compiler attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added define 'unused_' for '__attribute__((unused))'. The oldest version of GCC with online docs (GCC 2.95.3, released in March 16, 2001 [1]) already has support for this attribute, so it doesn't make sense to check the version. Renamed 'noreturn' to 'noreturn_' for consistency. [1] https://gcc.gnu.org/onlinedocs/ Signed-off-by: Antonio Niño Díaz --- include/asm/main.h | 2 +- include/extern/err.h | 8 ++++---- include/helpers.h | 6 ++++-- include/link/script.h | 2 +- src/asm/globlex.c | 4 +++- src/asm/symbol.c | 5 +++-- src/extern/err.c | 8 ++++---- src/link/lexer.l | 2 +- 8 files changed, 21 insertions(+), 16 deletions(-) diff --git a/include/asm/main.h b/include/asm/main.h index 77f3e854..49ab7add 100644 --- a/include/asm/main.h +++ b/include/asm/main.h @@ -46,7 +46,7 @@ void opt_Parse(char *s); * It is also used when the assembler goes into an invalid state (for example, * when it fails to allocate memory). */ -noreturn void fatalerror(const char *fmt, ...); +noreturn_ void fatalerror(const char *fmt, ...); /* * Used for errors that make it impossible to assemble correctly, but don't diff --git a/include/extern/err.h b/include/extern/err.h index 291e20a5..7a6e9e50 100644 --- a/include/extern/err.h +++ b/include/extern/err.h @@ -34,10 +34,10 @@ void vwarn(const char *fmt, va_list ap); void warnx(const char *fmt, ...); void vwarnx(const char *fmt, va_list ap); -noreturn void err(int status, const char *fmt, ...); -noreturn void verr(int status, const char *fmt, va_list ap); -noreturn void errx(int status, const char *fmt, ...); -noreturn void verrx(int status, const char *fmt, va_list ap); +noreturn_ void err(int status, const char *fmt, ...); +noreturn_ void verr(int status, const char *fmt, va_list ap); +noreturn_ void errx(int status, const char *fmt, ...); +noreturn_ void verrx(int status, const char *fmt, va_list ap); #endif /* ERR_IN_LIBC */ diff --git a/include/helpers.h b/include/helpers.h index 3a3cc476..3bf43aa0 100644 --- a/include/helpers.h +++ b/include/helpers.h @@ -11,10 +11,12 @@ #ifdef __GNUC__ /* GCC or compatible */ - #define noreturn __attribute__ ((noreturn)) + #define noreturn_ __attribute__ ((noreturn)) + #define unused_ __attribute__ ((unused)) #else /* Unsupported, but no need to throw a fit */ - #define noreturn + #define noreturn_ + #define unused_ #endif #endif /* HELPERS_H */ diff --git a/include/link/script.h b/include/link/script.h index 14e9b2f8..7a8663e0 100644 --- a/include/link/script.h +++ b/include/link/script.h @@ -13,7 +13,7 @@ #include "helpers.h" -noreturn void script_fatalerror(const char *fmt, ...); +noreturn_ void script_fatalerror(const char *fmt, ...); void script_Parse(const char *path); diff --git a/src/asm/globlex.c b/src/asm/globlex.c index b25afde0..d03d5b23 100644 --- a/src/asm/globlex.c +++ b/src/asm/globlex.c @@ -20,6 +20,8 @@ #include "asm/symbol.h" #include "asm/symbol.h" +#include "helpers.h" + #include "asmy.h" bool oDontExpandStrings; @@ -219,7 +221,7 @@ uint32_t PutMacroArg(char *src, uint32_t size) return 0; } -uint32_t PutUniqueArg(__attribute ((unused)) char *src, uint32_t size) +uint32_t PutUniqueArg(unused_ char *src, uint32_t size) { char *s; diff --git a/src/asm/symbol.c b/src/asm/symbol.c index 24bcb385..4f9f0c22 100644 --- a/src/asm/symbol.c +++ b/src/asm/symbol.c @@ -25,6 +25,7 @@ #include "extern/err.h" +#include "helpers.h" #include "version.h" struct sSymbol *tHashedSymbols[HASHSIZE]; @@ -62,7 +63,7 @@ void helper_RemoveLeadingZeros(char *string) memmove(string, new_beginning, strlen(new_beginning) + 1); } -int32_t Callback_NARG(__attribute__ ((unused)) struct sSymbol *sym) +int32_t Callback_NARG(unused_ struct sSymbol *sym) { uint32_t i = 0; @@ -72,7 +73,7 @@ int32_t Callback_NARG(__attribute__ ((unused)) struct sSymbol *sym) return i; } -int32_t Callback__LINE__(struct sSymbol __attribute__((unused)) *sym) +int32_t Callback__LINE__(unused_ struct sSymbol *sym) { return nLineNo; } diff --git a/src/extern/err.c b/src/extern/err.c index bcdde402..f1625132 100644 --- a/src/extern/err.c +++ b/src/extern/err.c @@ -33,7 +33,7 @@ void rgbds_vwarnx(const char *fmt, va_list ap) putc('\n', stderr); } -noreturn void rgbds_verr(int status, const char *fmt, va_list ap) +noreturn_ void rgbds_verr(int status, const char *fmt, va_list ap) { fprintf(stderr, "error"); if (fmt) { @@ -44,7 +44,7 @@ noreturn void rgbds_verr(int status, const char *fmt, va_list ap) exit(status); } -noreturn void rgbds_verrx(int status, const char *fmt, va_list ap) +noreturn_ void rgbds_verrx(int status, const char *fmt, va_list ap) { fprintf(stderr, "error"); if (fmt) { @@ -73,7 +73,7 @@ void rgbds_warnx(const char *fmt, ...) va_end(ap); } -noreturn void rgbds_err(int status, const char *fmt, ...) +noreturn_ void rgbds_err(int status, const char *fmt, ...) { va_list ap; @@ -82,7 +82,7 @@ noreturn void rgbds_err(int status, const char *fmt, ...) va_end(ap); } -noreturn void rgbds_errx(int status, const char *fmt, ...) +noreturn_ void rgbds_errx(int status, const char *fmt, ...) { va_list ap; diff --git a/src/link/lexer.l b/src/link/lexer.l index ac5a4fff..e0f65dc1 100644 --- a/src/link/lexer.l +++ b/src/link/lexer.l @@ -179,7 +179,7 @@ void script_PrintFileStack(void) fprintf(stderr, "%s(%d)", linkerscript_path, include_line[i]); } -noreturn void script_fatalerror(const char *fmt, ...) +noreturn_ void script_fatalerror(const char *fmt, ...) { va_list args; va_start(args, fmt);