shithub: rgbds

--- 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

--- a/CONTRIBUTING.rst

+++ b/CONTRIBUTING.rst

@@ -70,17 +70,22 @@

 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

@@ -87,7 +92,7 @@

    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.

--- a/Makefile

+++ b/Makefile

@@ -26,7 +26,7 @@

 VERSION_STRING	:= `git describe --tags --dirty --always 2>/dev/null`

-WARNFLAGS	:= -Wall -Werror

+WARNFLAGS	:= -Wall

 # Overridable CFLAGS

 CFLAGS		:= -g

@@ -160,6 +160,7 @@

 # 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";					\

@@ -169,6 +170,7 @@

 # 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		\

@@ -193,6 +195,20 @@

 	$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

@@ -200,7 +216,7 @@

 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

@@ -208,7 +224,7 @@

 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

--- a/include/asm/main.h

+++ b/include/asm/main.h

@@ -12,7 +12,7 @@

 #include <stdbool.h>

 #include <stdint.h>

-#include "extern/stdnoreturn.h"

+#include "helpers.h"

 struct sOptions {

 	char binary[2];

@@ -46,7 +46,7 @@

  * 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

--- a/include/extern/err.h

+++ b/include/extern/err.h

@@ -17,7 +17,7 @@

 #include <stdarg.h>

-#include "extern/stdnoreturn.h"

+#include "helpers.h"

 #define warn rgbds_warn

 #define vwarn rgbds_vwarn

@@ -34,10 +34,10 @@

 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 */

--- a/include/extern/stdnoreturn.h

+++ /dev/null

@@ -1,29 +1,0 @@

-/*

- * This file is part of RGBDS.

- *

- * Copyright (c) 2014-2018, RGBDS contributors.

- *

- * SPDX-License-Identifier: MIT

- */

-#ifndef EXTERN_STDNORETURN_H

-#define EXTERN_STDNORETURN_H

-#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))

-#elif _MSC_VER >= 1310

-	/* MS Visual Studio 2003/.NET Framework 1.1 or newer */

-	#define noreturn _declspec(noreturn)

-#else

-	/* Unsupported, but no need to throw a fit */

-	#define noreturn

-#endif

-#endif /* EXTERN_STDNORETURN_H */

--- /dev/null

+++ b/include/helpers.h

@@ -1,0 +1,22 @@

+/*

+ * This file is part of RGBDS.

+ *

+ * Copyright (c) 2014-2018, RGBDS contributors.

+ *

+ * SPDX-License-Identifier: MIT

+ */

+#ifndef HELPERS_H

+#define HELPERS_H

+#ifdef __GNUC__

+	/* GCC or compatible */

+	#define noreturn_	__attribute__ ((noreturn))

+	#define unused_		__attribute__ ((unused))

+#else

+	/* Unsupported, but no need to throw a fit */

+	#define noreturn_

+	#define unused_

+#endif

+#endif /* HELPERS_H */

--- a/include/link/script.h

+++ b/include/link/script.h

@@ -11,9 +11,9 @@

 #include <stdint.h>

-#include "extern/stdnoreturn.h"

+#include "helpers.h"

-noreturn void script_fatalerror(const char *fmt, ...);

+noreturn_ void script_fatalerror(const char *fmt, ...);

 void script_Parse(const char *path);

--- a/src/asm/asmy.y

+++ b/src/asm/asmy.y

@@ -341,8 +341,9 @@

 				break;

 			case '\"':

-				src++;

+				src += 2;

 				inString = false;

+				break;

 			default:

 				src++;

--- a/src/asm/fstack.c

+++ b/src/asm/fstack.c

@@ -87,6 +87,8 @@

 		(*ppFileStack)->nREPTBlockSize = nCurrentREPTBlockSize;

 		(*ppFileStack)->nREPTBlockCount = nCurrentREPTBlockCount;

 		break;

+	default:

+		fatalerror("%s: Internal error.", __func__);

 	nLineNo = 0;

@@ -152,6 +154,8 @@

 		nCurrentREPTBlockSize = pLastFile->nREPTBlockSize;

 		nCurrentREPTBlockCount = pLastFile->nREPTBlockCount;

 		break;

+	default:

+		fatalerror("%s: Internal error.", __func__);

 	free(*ppLastFile);

@@ -174,6 +178,8 @@

 		return nLineNo; /* ??? */

 	case STAT_isREPTBlock:

 		break; /* Peek top file of the stack */

+	default:

+		fatalerror("%s: Internal error.", __func__);

 	pLastFile = pFileStack;

@@ -257,8 +263,10 @@

 		 * 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");

--- 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;

@@ -93,6 +95,9 @@

 		s += 1;

 		convertfunc = binary2bin;

 		break;

+	default:

+		/* Handle below */

+		break;

 	if (radix == 4) {

@@ -216,7 +221,7 @@

 	return 0;

-uint32_t PutUniqueArg(char *src, uint32_t size)

+uint32_t PutUniqueArg(unused_ char *src, uint32_t size)

 	char *s;

--- a/src/asm/lexer.c

+++ b/src/asm/lexer.c

@@ -414,7 +414,7 @@

/*

  * 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;

@@ -640,7 +640,6 @@

 	/* 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

@@ -851,7 +850,7 @@

 		return yylex_NORMAL();

 	case LEX_STATE_MACROARGS:

 		return yylex_MACROARGS();

+	default:

+		fatalerror("%s: Internal error.", __func__);

-	fatalerror("Internal error in %s", __func__);

--- a/src/asm/main.c

+++ b/src/asm/main.c

@@ -6,6 +6,7 @@

  * SPDX-License-Identifier: MIT

*/

+#include <float.h>

 #include <math.h>

 #include <stdarg.h>

 #include <stdio.h>

@@ -147,10 +148,13 @@

 	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'");

@@ -222,7 +226,7 @@

 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]);

@@ -480,7 +484,7 @@

 	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",

--- 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 @@

 	memmove(string, new_beginning, strlen(new_beginning) + 1);

-int32_t Callback_NARG(struct sSymbol *sym)

+int32_t Callback_NARG(unused_ struct sSymbol *sym)

 	uint32_t i = 0;

@@ -72,7 +73,7 @@

 	return i;

-int32_t Callback__LINE__(struct sSymbol __attribute__((unused)) *sym)

+int32_t Callback__LINE__(unused_ struct sSymbol *sym)

 	return nLineNo;

@@ -673,7 +674,7 @@

 			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",

--- a/src/extern/err.c

+++ b/src/extern/err.c

@@ -33,7 +33,7 @@

 	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 @@

 	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 @@

 	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 @@

 	va_end(ap);

-noreturn void rgbds_errx(int status, const char *fmt, ...)

+noreturn_ void rgbds_errx(int status, const char *fmt, ...)

 	va_list ap;

--- a/src/gfx/main.c

+++ b/src/gfx/main.c

@@ -53,6 +53,7 @@

 			break;

 		case 'F':

 			opts.hardfix = true;

+			/* fallthrough */

 		case 'f':

 			opts.fix = true;

 			break;

--- a/src/gfx/makepng.c

+++ b/src/gfx/makepng.c

@@ -317,9 +317,9 @@

 			     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,

@@ -469,8 +469,10 @@

 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;

--- a/src/link/assign.c

+++ b/src/link/assign.c

@@ -74,6 +74,10 @@

 		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 AssignSections(void)

-	int32_t i;

 	struct sSection *pSection;

 	MaxBankUsed = 0;

@@ -503,7 +506,7 @@

 	 * 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]) {

@@ -617,6 +620,9 @@

 				     pSection->nOrg, pSection->nBank);

 			break;

+		default:

+			errx(1, "%s: Internal error: Type %d", __func__,

+			     pSection->Type);

@@ -660,6 +666,10 @@

 			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;

--- 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. */

@@ -179,7 +179,7 @@

 	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);

--- 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 @@

%%

-extern int yylex();

-extern int yyparse();

+extern int yylex(void);

+extern int yyparse(void);

 int yywrap(void)

--- a/src/link/patch.c

+++ b/src/link/patch.c

@@ -325,6 +325,8 @@

 					     pPatch->nLineNo);

 				break;

+			default:

+				errx(1, "%s: Internal error.", __func__);

 			pPatch = pPatch->pNext;

--- a/src/link/script.c

+++ b/src/link/script.c

@@ -85,34 +85,34 @@

-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;

@@ -119,25 +119,25 @@

 		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 @@

 		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__);

--

⑨