Fix RGBFIX --output with a pre-existing or same-as-input file (#2145)

include/platform.hpp

@@ -53,6 +53,12 @@
 	#define O_TEXT   0   // Assume that it's not defined either
 #endif                   // _MSC_VER
 
+// MSVC doesn't have POSIX `ftruncate`, use a suitable replacement
+#if defined(_MSC_VER)
+	#include <io.h> // IWYU pragma: export
+	#define ftruncate _chsize_s
+#endif
+
 // Windows has stdin and stdout open as text by default, which we may not want
 #if defined(_MSC_VER) || defined(__MINGW32__)
 	#include <io.h> // IWYU pragma: export

src/fix/fix.cpp

@@ -381,6 +381,24 @@ static void
 		if (options.padValue == UNSPECIFIED) {
 			rom0Len = headerSize;
 		}
+	} else if (output != STDOUT_FILENO) {
+		// Truncate a pre-existing output file before writing to it
+		if (ftruncate(output, 0) != 0) {
+			// LCOV_EXCL_START
+			int errnum = errno;
+			if (errnum == EINVAL) {
+				// We do not support FIFOs or symlinks
+				if (struct stat stat; fstat(output, &stat) == 0 && !S_ISREG(stat.st_mode)) {
+					error(
+					    "\"%s\" is not a regular file, and thus cannot be modified in-place", name
+					);
+					return;
+				}
+			}
+			error("Failed to truncate \"%s\": %s", name, strerror(errnum));
+			return;
+			// LCOV_EXCL_STOP
+		}
 	}
 	writeLen = writeBytes(output, rom0, rom0Len);
 
@@ -498,8 +516,7 @@ bool fix_ProcessFile(char const *name, char const *outputName) {
 		error("Failed to open \"%s\" for reading+writing: %s", name, strerror(errno));
 	} else {
 		Defer closeInput{[&] { xclose(input); }};
-		struct stat stat;
+		if (struct stat stat; fstat(input, &stat) == -1) {
-		if (fstat(input, &stat) == -1) {
 			error("Failed to stat \"%s\": %s", name, strerror(errno)); // LCOV_EXCL_LINE
 		} else if (!S_ISREG(stat.st_mode)) { // We do not support FIFOs or symlinks
 			// LCOV_EXCL_START

test/fix/test.sh

@@ -44,7 +44,7 @@ tryDiff () {
 
 tryCmp () {
 	if ! cmp "$1" "$2"; then
-		"$src/../../gbdiff.bash" "$1" "$2"
+		"$src/../../contrib/gbdiff.bash" "$1" "$2" | head
 		echo "${bold}${red}${3:-$1} mismatch!${rescolors}${resbold}"
 		false
 	fi
@@ -130,6 +130,26 @@ for i in "$src"/*.flags; do
 	runTest "$(basename "$i" .flags)" "$src"
 done
 
+# Check that RGBFIX truncates a pre-existing output file
+name=pre-existing-output
+echo "${bold}${green}${name}...${rescolors}${resbold}"
+dd if=/dev/zero of=input.gb bs=1 count=336 >/dev/null 2>&1
+dd if=/dev/zero of=out.gb bs=1 count=16384 >/dev/null 2>&1
+eval "$RGBFIX" -o out.gb input.gb '>out.out' '2>out.err'
+tryDiff out.out /dev/null "${name}.out"
+tryDiff out.err /dev/null "${name}.err"
+tryCmp input.gb out.gb "${name}.gb"
+
+# Check that RGBFIX handles an output file identical to the input file
+name=equivalent-output
+echo "${bold}${green}${name}...${rescolors}${resbold}"
+dd if=/dev/zero of=input.gb bs=1 count=336 >/dev/null 2>&1
+cp input.gb out.gb
+eval "$RGBFIX" -o out.gb out.gb '>out.out' '2>out.err'
+tryDiff out.out /dev/null "${name}.out"
+tryDiff out.err /dev/null "${name}.err"
+tryCmp input.gb out.gb "${name}.gb"
+
 # Check that RGBFIX errors out when inputting a non-existent file
 runSpecialTest no-exist no-exist
 
@@ -149,7 +169,8 @@ for (( i=0; i < 10; ++i )); do
 	echo "$padding..."
 	for suffix in '' -large -larger; do
 		cat <<<"-p $padding" >padding$suffix.flags
-		tr '\377' \\$((padding / 64))$(((padding / 8) % 8))$((padding % 8)) <"$src/padding$suffix.gb" >padding$suffix.gb # OK because $FF bytes are only used for padding
+		# `tr`ing &377 aka $FF is OK because $FF bytes are only used for padding
+		tr '\377' \\$((padding / 64))$(((padding / 8) % 8))$((padding % 8)) <"$src/padding$suffix.gb" >padding$suffix.gb
 		runTest padding${suffix} .
 	done
 done