Allow padding to coexist with overlay file (#1395)

include/link/main.hpp

@@ -20,6 +20,7 @@ extern char const *symFileName;
 extern char const *overlayFileName;
 extern char const *outputFileName;
 extern uint8_t padValue;
+extern bool hasPadValue;
 extern uint16_t scrambleROMX;
 extern uint8_t scrambleWRAMX;
 extern uint8_t scrambleSRAM;

man/rgblink.1

@@ -93,9 +93,6 @@ address)!
 Write the ROM image to the given file.
 .It Fl p Ar pad_value , Fl \-pad Ar pad_value
 When inserting padding between sections, pad with this value.
-Has no effect if
-.Fl O
-is specified.
 The default is 0.
 .It Fl S Ar spec , Fl \-scramble Ar spec
 Enables a different

src/asm/section.cpp

@@ -846,15 +846,14 @@ void sect_BinaryFile(std::string const &name, int32_t startPos) {
 			if (verbose)
 				printf("Aborting (-MG) on INCBIN file '%s' (%s)\n", name.c_str(), strerror(errno));
 			failedOnMissingInclude = true;
-			return;
+		} else {
+			error("Error opening INCBIN file '%s': %s\n", name.c_str(), strerror(errno));
 		}
-		error("Error opening INCBIN file '%s': %s\n", name.c_str(), strerror(errno));
 		return;
 	}
 	Defer closeFile{[&] { fclose(file); }};
 
 	int32_t fsize = -1;
-	int byte;
 
 	if (fseek(file, 0, SEEK_END) != -1) {
 		fsize = ftell(file);
@@ -865,6 +864,7 @@ void sect_BinaryFile(std::string const &name, int32_t startPos) {
 		}
 
 		fseek(file, startPos, SEEK_SET);
+
 		if (!reserveSpace(fsize - startPos))
 			return;
 	} else {
@@ -877,7 +877,7 @@ void sect_BinaryFile(std::string const &name, int32_t startPos) {
 			(void)fgetc(file);
 	}
 
-	while ((byte = fgetc(file)) != EOF) {
+	for (int byte; (byte = fgetc(file)) != EOF;) {
 		if (fsize == -1)
 			growSection(1);
 		writebyte(byte);
@@ -887,6 +887,7 @@ void sect_BinaryFile(std::string const &name, int32_t startPos) {
 		error("Error reading INCBIN file '%s': %s\n", name.c_str(), strerror(errno));
 }
 
+// Output a slice of a binary file
 void sect_BinaryFileSlice(std::string const &name, int32_t startPos, int32_t length) {
 	if (startPos < 0) {
 		error("Start position cannot be negative (%" PRId32 ")\n", startPos);
@@ -920,10 +921,8 @@ void sect_BinaryFileSlice(std::string const &name, int32_t startPos, int32_t len
 	}
 	Defer closeFile{[&] { fclose(file); }};
 
-	int32_t fsize;
-
 	if (fseek(file, 0, SEEK_END) != -1) {
-		fsize = ftell(file);
+		int32_t fsize = ftell(file);
 
 		if (startPos > fsize) {
 			error("Specified start position is greater than length of file\n");

src/link/main.cpp

@@ -32,6 +32,7 @@ char const *symFileName;     // -n
 char const *overlayFileName; // -O
 char const *outputFileName;  // -o
 uint8_t padValue;            // -p
+bool hasPadValue = false;
 // Setting these three to 0 disables the functionality
 uint16_t scrambleROMX = 0; // -S
 uint8_t scrambleWRAMX = 0;
@@ -387,6 +388,7 @@ int main(int argc, char *argv[]) {
 				value = 0xFF;
 			}
 			padValue = value;
+			hasPadValue = true;
 			break;
 		}
 		case 'S':

src/link/output.cpp

@@ -116,17 +116,13 @@ static uint32_t checkOverlaySize() {
 	fseek(overlayFile, 0, SEEK_SET);
 
 	if (overlaySize % BANK_SIZE)
-		errx("Overlay file must have a size multiple of 0x4000");
+		warnx("Overlay file does not have a size multiple of 0x4000");
+	else if (is32kMode && overlaySize != 0x8000)
+		warnx("Overlay is not exactly 0x8000 bytes large");
+	else if (overlaySize < 0x8000)
+		warnx("Overlay is less than 0x8000 bytes large");
 
-	uint32_t nbOverlayBanks = overlaySize / BANK_SIZE;
+	return (overlaySize + BANK_SIZE - 1) / BANK_SIZE;
-
-	if (is32kMode && nbOverlayBanks != 2)
-		errx("Overlay must be exactly 0x8000 bytes large");
-
-	if (nbOverlayBanks < 2)
-		errx("Overlay must be at least 0x8000 bytes large");
-
-	return nbOverlayBanks;
 }
 
 /*
@@ -149,6 +145,22 @@ static void coverOverlayBanks(uint32_t nbOverlayBanks) {
 	}
 }
 
+static uint8_t getNextFillByte() {
+	if (overlayFile) {
+		int c = getc(overlayFile);
+		if (c != EOF) {
+			return c;
+		}
+
+		if (static bool warned = false; !hasPadValue && !warned) {
+			warnx("Output is larger than overlay file, but no padding value was specified");
+			warned = true;
+		}
+	}
+
+	return padValue;
+}
+
 /*
  * Write a ROM bank's sections to the output file.
  * @param bankSections The bank's sections, ordered by increasing address
@@ -164,7 +176,7 @@ static void
 			assume(section->offset == 0);
 			// Output padding up to the next SECTION
 			while (offset + baseOffset < section->org) {
-				putc(overlayFile ? getc(overlayFile) : padValue, outputFile);
+				putc(getNextFillByte(), outputFile);
 				offset++;
 			}
 
@@ -181,7 +193,7 @@ static void
 
 	if (!disablePadding) {
 		while (offset < size) {
-			putc(overlayFile ? getc(overlayFile) : padValue, outputFile);
+			putc(getNextFillByte(), outputFile);
 			offset++;
 		}
 	}

test/link/overlay/smaller/a.asm

@@ -0,0 +1,9 @@
+SECTION "start", ROM0[0]
+db 1, 2, 3, 4
+ds 4
+db 5, 6, 7, 8
+
+SECTION "end", ROMX[$4000], BANK[2]
+db 9, 10, 11, 12
+ds 4
+db 13, 14, 15, 16

test/link/overlay/smaller/out.err

@@ -0,0 +1 @@
+warning: Overlay file does not have a size multiple of 0x4000

test/link/test.sh

@@ -181,7 +181,17 @@ rgblinkQuiet -o "$gbtemp2" "$outtemp"
 tryCmp "$gbtemp" "$gbtemp2"
 evaluateTest
 
-test="overlay"
+test="overlay/smaller"
+startTest
+"$RGBASM" -o "$otemp" "$test"/a.asm
+continueTest
+rgblinkQuiet -o "$gbtemp" -p 0x42 -O "$test"/overlay.gb "$otemp" 2>"$outtemp"
+tryDiff "$test"/out.err "$outtemp"
+# This test does not trim its output with 'dd' because it needs to verify the correct output size
+tryCmp "$test"/out.gb "$gbtemp"
+evaluateTest
+
+test="overlay/tiny"
 startTest
 "$RGBASM" -o "$otemp" "$test"/a.asm
 continueTest