shithub: rgbds

Info  •  Files  •  Log  •  Branches

Fix a bunch of Clang warnings

As reported by #789
Should avoid relying on 32-bit int (for implicit conversions)
and account for more extreme uses of RGBDS.

--- a/include/asm/symbol.h

+++ b/include/asm/symbol.h

@@ -75,7 +75,7 @@

 	if (sym->type == SYM_LABEL) {

 		struct Section const *sect = sym_GetSection(sym);

-		return sect && sect->org != -1;

+		return sect && sect->org != (uint32_t)-1;

 	}

 	return sym->type == SYM_EQU || sym->type == SYM_SET;

 }

--- a/include/link/section.h

+++ b/include/link/section.h

@@ -30,11 +30,11 @@

 struct Patch {

 	struct FileStackNode const *src;

 	uint32_t lineNo;

-	int32_t offset;

+	uint32_t offset;

 	uint32_t pcSectionID;

 	uint32_t pcOffset;

 	enum PatchType type;

-	int32_t rpnSize;

+	uint32_t rpnSize;

 	uint8_t *rpnExpression;

 	struct Section const *pcSection;

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

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

@@ -175,8 +175,14 @@

 			char const *incPath = i ? includePaths[i - 1] : "";

 			int len = snprintf(*fullPath, *size, "%s%s", incPath, path);

+			if (len < 0) {

+				error("snprintf error during include path search: %s\n",

+				      strerror(errno));

+				break;

+			}

+

 			/* Oh how I wish `asnprintf` was standard... */

-			if (len >= *size) { /* `len` doesn't include the terminator, `size` does */

+			if ((size_t)len >= *size) { /* `len` doesn't include the terminator, `size` does */

 				*size = len + 1;

 				*fullPath = realloc(*fullPath, *size);

 				if (!*fullPath) {

@@ -187,10 +193,7 @@

 				len = sprintf(*fullPath, "%s%s", incPath, path);

 			}

-			if (len < 0) {

-				error("snprintf error during include path search: %s\n",

-				      strerror(errno));

-			} else if (isPathValid(*fullPath)) {

+			if (isPathValid(*fullPath)) {

 				printDep(*fullPath);

 				return true;

 			}

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

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

@@ -866,8 +866,9 @@

 			size_t nbExpectedChars = LEXER_BUF_SIZE - writeIndex;

 			readChars(nbExpectedChars);

-			/* If the read was incomplete, don't perform a second read */

-			if (nbCharsRead < nbExpectedChars)

+			// If the read was incomplete, don't perform a second read

+			// `nbCharsRead` cannot be negative, so it's fine to cast to `size_t`

+			if ((size_t)nbCharsRead < nbExpectedChars)

 				target = 0;

 		}

 		if (target != 0)

@@ -2258,7 +2259,7 @@

 {

 	dbgPrint("Skipping IF block (toEndc = %s)\n", toEndc ? "true" : "false");

 	lexer_SetMode(LEXER_NORMAL);

-	int startingDepth = lexer_GetIFDepth();

+	uint32_t startingDepth = lexer_GetIFDepth();

 	int token;

 	bool atLineStart = lexerState->atLineStart;

--- a/src/asm/macro.c

+++ b/src/asm/macro.c

@@ -170,12 +170,12 @@

 {

 	if (!macroArgs) {

 		error("Cannot shift macro arguments outside of a macro\n");

-	} else if (count > 0 && (count > macroArgs->nbArgs

+	} else if (count > 0 && ((uint32_t)count > macroArgs->nbArgs

 		   || macroArgs->shift > macroArgs->nbArgs - count)) {

 		warning(WARNING_MACRO_SHIFT,

 			"Cannot shift macro arguments past their end\n");

 		macroArgs->shift = macroArgs->nbArgs;

-	} else if (count < 0 && macroArgs->shift < -count) {

+	} else if (count < 0 && macroArgs->shift < (uint32_t)-count) {

 		warning(WARNING_MACRO_SHIFT,

 			"Cannot shift macro arguments past their beginning\n");

 		macroArgs->shift = 0;

--- a/src/asm/output.c

+++ b/src/asm/output.c

@@ -136,9 +136,9 @@

 void out_RegisterNode(struct FileStackNode *node)

 {

 	/* If node is not already registered, register it (and parents), and give it a unique ID */

-	while (node->ID == -1) {

+	while (node->ID == (uint32_t)-1) {

 		node->ID = getNbFileStackNodes();

-		if (node->ID == -1)

+		if (node->ID == (uint32_t)-1)

 			fatalerror("Reached too many file stack nodes; try splitting the file up\n");

 		node->next = fileStackNodes;

 		fileStackNodes = node;

@@ -266,7 +266,7 @@

 	*objectSymbolsTail = sym;

 	objectSymbolsTail = &sym->next;

 	out_RegisterNode(sym->src);

-	if (nbSymbols == -1)

+	if (nbSymbols == (uint32_t)-1)

 		fatalerror("Registered too many symbols (%" PRIu32

 			   "); try splitting up your files\n", (uint32_t)-1);

 	sym->ID = nbSymbols++;

@@ -278,7 +278,7 @@

  */

 static uint32_t getSymbolID(struct Symbol *sym)

 {

-	if (sym->ID == -1 && !sym_IsPC(sym))

+	if (sym->ID == (uint32_t)-1 && !sym_IsPC(sym))

 		registerSymbol(sym);

 	return sym->ID;

 }

@@ -474,7 +474,7 @@

 	(void)arg; // sym_ForEach requires a void* parameter, but we are not using it.

 	// Check for symbol->src, to skip any built-in symbol from rgbasm

-	if (symbol->src && symbol->ID == -1) {

+	if (symbol->src && symbol->ID == (uint32_t)-1) {

 		registerSymbol(symbol);

 	}

 }

--- a/src/asm/parser.y

+++ b/src/asm/parser.y

@@ -176,7 +176,9 @@

 	for (char const *next = strstr(src, old); next && *next; next = strstr(src, old)) {

 		// Copy anything before the substring to replace

-		memcpy(dest + i, src, next - src < destLen - i ? next - src : destLen - i);

+		unsigned int lenBefore = next - src;

+

+		memcpy(dest + i, src, lenBefore < destLen - i ? lenBefore : destLen - i);

 		i += next - src;

 		if (i >= destLen)

 			break;

@@ -1437,7 +1439,11 @@

 strcat_args	: string

 		| strcat_args T_COMMA string {

-			if (snprintf($$, sizeof($$), "%s%s", $1, $3) >= sizeof($$))

+			int ret = snprintf($$, sizeof($$), "%s%s", $1, $3);

+

+			if (ret == -1)

+				fatalerror("snprintf error in STRCAT: %s\n", strerror(errno));

+			else if ((unsigned int)ret >= sizeof($$))

 				warning(WARNING_LONG_STR, "STRCAT: String too long '%s%s'\n",

 					$1, $3);

 		}

--- a/src/asm/rpn.c

+++ b/src/asm/rpn.c

@@ -139,7 +139,7 @@

 	if (!pCurrentSection) {

 		error("PC has no bank outside a section\n");

 		expr->nVal = 1;

-	} else if (pCurrentSection->bank == -1) {

+	} else if (pCurrentSection->bank == (uint32_t)-1) {

 		makeUnknown(expr, "Current section's bank is not known");

 		expr->nRPNPatchSize++;

 		*reserveSpace(expr, 1) = RPN_BANK_SELF;

@@ -165,7 +165,7 @@

 		sym = sym_Ref(tzSym);

 		assert(sym); // If the symbol didn't exist, it should have been created

-		if (sym_GetSection(sym) && sym_GetSection(sym)->bank != -1) {

+		if (sym_GetSection(sym) && sym_GetSection(sym)->bank != (uint32_t)-1) {

 			/* Symbol's section is known and bank is fixed */

 			expr->nVal = sym_GetSection(sym)->bank;

 		} else {

@@ -186,7 +186,7 @@

 	struct Section *pSection = out_FindSectionByName(tzSectionName);

-	if (pSection && pSection->bank != -1) {

+	if (pSection && pSection->bank != (uint32_t)-1) {

 		expr->nVal = pSection->bank;

 	} else {

 		makeUnknown(expr, "Section \"%s\"'s bank is not known",

--- a/src/asm/section.c

+++ b/src/asm/section.c

@@ -114,9 +114,9 @@

 	if (sect_HasData(type))

 		fail("Cannot declare ROM sections as UNION\n");

-	if (org != -1) {

+	if (org != (uint32_t)-1) {

 		/* If both are fixed, they must be the same */

-		if (sect->org != -1 && sect->org != org)

+		if (sect->org != (uint32_t)-1 && sect->org != org)

 			fail("Section already declared as fixed at different address $%04"

 			     PRIx32 "\n", sect->org);

 		else if (sect->align != 0 && (mask(sect->align) & (org - sect->alignOfs)))

@@ -128,7 +128,7 @@

 	} else if (alignment != 0) {

 		/* Make sure any fixed address given is compatible */

-		if (sect->org != -1) {

+		if (sect->org != (uint32_t)-1) {

 			if ((sect->org - alignOffset) & mask(alignment))

 				fail("Section already declared as fixed at incompatible address $%04"

 				     PRIx32 "\n", sect->org);

@@ -160,11 +160,11 @@

 	 * combination of both.

 	 * The merging is however performed at the *end* of the original section!

 	 */

-	if (org != -1) {

+	if (org != (uint32_t)-1) {

 		uint16_t curOrg = org - sect->size;

 		/* If both are fixed, they must be the same */

-		if (sect->org != -1 && sect->org != curOrg)

+		if (sect->org != (uint32_t)-1 && sect->org != curOrg)

 			fail("Section already declared as fixed at incompatible address $%04"

 			     PRIx32 " (cur addr = %04" PRIx32 ")\n",

 			     sect->org, sect->org + sect->size);

@@ -182,7 +182,7 @@

 			curOfs += 1U << alignment;

 		/* Make sure any fixed address given is compatible */

-		if (sect->org != -1) {

+		if (sect->org != (uint32_t)-1) {

 			if ((sect->org - curOfs) & mask(alignment))

 				fail("Section already declared as fixed at incompatible address $%04"

 				     PRIx32 "\n", sect->org);

@@ -221,10 +221,10 @@

 			// Common checks

 			/* If the section's bank is unspecified, override it */

-			if (sect->bank == -1)

+			if (sect->bank == (uint32_t)-1)

 				sect->bank = bank;

 			/* If both specify a bank, it must be the same one */

-			else if (bank != -1 && sect->bank != bank)

+			else if (bank != (uint32_t)-1 && sect->bank != bank)

 				fail("Section already declared with different bank %" PRIu32 "\n",

 				     sect->bank);

 			break;

@@ -296,7 +296,7 @@

 	// First, validate parameters, and normalize them if applicable

-	if (bank != -1) {

+	if (bank != (uint32_t)-1) {

 		if (type != SECTTYPE_ROMX && type != SECTTYPE_VRAM

 		 && type != SECTTYPE_SRAM && type != SECTTYPE_WRAMX)

 			error("BANK only allowed for ROMX, WRAMX, SRAM, or VRAM sections\n");

@@ -316,7 +316,7 @@

 		alignOffset = 0;

 	}

-	if (org != -1) {

+	if (org != (uint32_t)-1) {

 		if (org < startaddr[type] || org > endaddr(type))

 			error("Section \"%s\"'s fixed address %#" PRIx32

 				" is outside of range [%#" PRIx16 "; %#" PRIx16 "]\n",

@@ -331,7 +331,7 @@

 		/* It doesn't make sense to have both alignment and org set */

 		uint32_t mask = mask(alignment);

-		if (org != -1) {

+		if (org != (uint32_t)-1) {

 			if ((org - alignOffset) & mask)

 				error("Section \"%s\"'s fixed address doesn't match its alignment\n",

 					name);

@@ -453,7 +453,7 @@

 	struct Section *sect = sect_GetSymbolSection();

 	uint16_t alignSize = 1 << alignment; // Size of an aligned "block"

-	if (sect->org != -1) {

+	if (sect->org != (uint32_t)-1) {

 		if ((sym_GetPCValue() - offset) % alignSize)

 			error("Section's fixed address fails required alignment (PC = $%04" PRIx32

 			      ")\n", sym_GetPCValue());

--- a/src/asm/symbol.c

+++ b/src/asm/symbol.c

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

 	setSymbolFilename(sym);

 	/* If the old node was referenced, ensure the new one is */

-	if (oldSrc && oldSrc->referenced && oldSrc->ID != -1)

+	if (oldSrc && oldSrc->referenced && oldSrc->ID != (uint32_t)-1)

 		out_RegisterNode(sym->src);

 	/* TODO: unref the old node, and use `out_ReplaceNode` instead if deleting it */

 }

@@ -277,7 +277,7 @@

 static inline bool isReferenced(struct Symbol const *sym)

 {

-	return sym->ID != -1;

+	return sym->ID != (uint32_t)-1;

 }

 /*

@@ -315,7 +315,7 @@

 	if (!sect)

 		error("PC has no value outside a section\n");

-	else if (sect->org == -1)

+	else if (sect->org == (uint32_t)-1)

 		error("Expected constant PC but section is not fixed\n");

 	else

 		return CallbackPC();

--- a/src/asm/util.c

+++ b/src/asm/util.c

@@ -57,7 +57,7 @@

 	default: /* Print as hex */

 		buf[1] = 'x';

-		sprintf(&buf[2], "%02hhx", c);

+		sprintf(&buf[2], "%02hhx", (uint8_t)c);

 		return buf;

 	}

 	buf[2] = '\0';

--- a/src/fix/main.c

+++ b/src/fix/main.c

@@ -870,11 +870,13 @@

 	// Output ROMX if it was buffered

 	if (romx) {

+		// The value returned is either -1, or smaller than `totalRomxLen`,

+		// so it's fine to cast to `size_t`

 		writeLen = writeBytes(output, romx, totalRomxLen);

 		if (writeLen == -1) {

 			report("FATAL: Failed to write \"%s\"'s ROMX: %s\n", name, strerror(errno));

 			goto free_romx;

-		} else if (writeLen < totalRomxLen) {

+		} else if ((size_t)writeLen < totalRomxLen) {

 			report("FATAL: Could only write %ld of \"%s\"'s %ld ROMX bytes\n",

 			       writeLen, name, totalRomxLen);

 			goto free_romx;

@@ -898,7 +900,9 @@

 			size_t thisLen = len > sizeof(bank) ? sizeof(bank) : len;

 			ssize_t ret = writeBytes(output, bank, thisLen);

-			if (ret != thisLen) {

+			// The return value is either -1, or at most `thisLen`,

+			// so it's fine to cast to `size_t`

+			if ((size_t)ret != thisLen) {

 				report("FATAL: Failed to write \"%s\"'s padding: %s\n",

 				       name, strerror(errno));

 				break;

--- a/src/link/object.c

+++ b/src/link/object.c

@@ -188,7 +188,7 @@

 	tryReadlong(parentID, file,

 		    "%s: Cannot read node #%" PRIu32 "'s parent ID: %s", fileName, i);

-	fileNodes[i].parent = parentID == -1 ? NULL : &fileNodes[parentID];

+	fileNodes[i].parent = parentID == (uint32_t)-1 ? NULL : &fileNodes[parentID];

 	tryReadlong(fileNodes[i].lineNo, file,

 		    "%s: Cannot read node #%" PRIu32 "'s line number: %s", fileName, i);

 	tryGetc(fileNodes[i].type, file, "%s: Cannot read node #%" PRIu32 "'s type: %s",

@@ -279,7 +279,8 @@

 	tryReadlong(patch->pcSectionID, file,

 		    "%s: Unable to read \"%s\"'s patch #%" PRIu32 "'s PC offset: %s",

 		    fileName, sectName, i);

-	patch->pcSection = patch->pcSectionID == -1 ? NULL : fileSections[patch->pcSectionID];

+	patch->pcSection = patch->pcSectionID == (uint32_t)-1 ? NULL

+							      : fileSections[patch->pcSectionID];

 	tryReadlong(patch->pcOffset, file,

 		    "%s: Unable to read \"%s\"'s patch #%" PRIu32 "'s PC offset: %s",

 		    fileName, sectName, i);

@@ -635,7 +636,7 @@

 	free(section->name);

 	if (sect_HasData(section->type)) {

 		free(section->data);

-		for (int32_t i = 0; i < section->nbPatches; i++)

+		for (uint32_t i = 0; i < section->nbPatches; i++)

 			free(section->patches[i].rpnExpression);

 		free(section->patches);

 	}

-- 

⑨