Implement warning diagnostic flags for RGBFIX (#1766)

* Implement warning diagnostic flags for RGBFIX

* `-m/--mbc-type help` prints to stdout

* Support `-m list` as well as `-m help`

* Make invalid `rgbfix -l` characters a fatal error, like other invalid CLI arguments

* Refactor fix/main.cpp into multiple files

CONTRIBUTING.md

@@ -97,7 +97,7 @@ All tests begin by assembling the `.asm` file into an object file, which will be
 
 These simply check that RGBLINK's output matches some expected output.
 
-A `.out` file **must** exist, and RGBLINK's output must match that file's contents.
+A `.out` file **must** exist, and RGBLINK's total output must match that file's contents.
 
 Additionally, if a `.out.bin` file exists, the `.gb` file generated by RGBLINK must match it.
 
@@ -106,14 +106,14 @@ Additionally, if a `.out.bin` file exists, the `.gb` file generated by RGBLINK m
 These allow applying various linker scripts to the same object file.
 If one or more `.link` files exist, whose names start the same as the `.asm` file, then each of those files correspond to one test.
 
-Each `.link` linker script **must** be accompanied by a `.out` file, and RGBLINK's output must match that file's contents when passed the corresponding linker script.
+Each `.link` linker script **must** be accompanied by a `.out` file, and RGBLINK's total output must match that file's contents when passed the corresponding linker script.
 
 #### Variant tests
 
 These allow testing RGBLINK's `-d`, `-t`, and `-w` flags.
 If one or more <code>-<var>&lt;flag&gt;</var>.out</code> or <code>-no-<var>&lt;flag&gt;</var>.out</code> files exist, then each of them corresponds to one test.
 
-The object file will be linked with and without said flag, respectively; and in each case, RGBLINK's output must match the `.out` file's contents.
+The object file will be linked with and without said flag, respectively; and in each case, RGBLINK's total output must match the `.out` file's contents.
 
 ### RGBFIX
 
@@ -123,8 +123,11 @@ Each one is a text file whose first line contains flags to pass to RGBFIX.
 
 RGBFIX will be invoked on the `.bin` file if it exists, or else on default-input.bin.
 
+If no `.out` file exist, RGBFIX is not expected to output anything.
+If one *does* exist, RGBFIX's output **must** match the `.out` file's contents.
+
 If no `.err` file exists, RGBFIX is simply expected to be able to process the file normally.
-If one *does* exist, RGBFIX's return status is ignored, but its output **must** match the `.err` file's contents.
+If one *does* exist, RGBFIX's return status is ignored, but its error output **must** match the `.err` file's contents.
 
 Additionally, if a `.gb` file exists, the output of RGBFIX must match the `.gb`.
 

Makefile

@@ -97,7 +97,9 @@ src/link/main.o: src/link/script.hpp
 
 rgbfix_obj := \
 	${common_obj} \
-	src/fix/main.o
+	src/fix/main.o \
+	src/fix/mbc.o \
+	src/fix/warning.o
 
 rgbgfx_obj := \
 	${common_obj} \

contrib/bash_compl/_rgbasm.bash

@@ -24,13 +24,13 @@ _rgbasm_completions() {
 	# Empty long opt = it doesn't exit
 	# See the `state` variable below for info about `state_after`
 	declare -A opts=(
-		[V]="version:normal"
 		[h]="help:normal"
-		[E]="export-all:normal"
+		[V]="version:normal"
-		[v]="verbose:normal"
+		[W]="warning:warning"
 		[w]=":normal"
 		[b]="binary-digits:unk"
 		[D]="define:unk"
+		[E]="export-all:normal"
 		[g]="gfx-chars:unk"
 		[I]="include:dir"
 		[M]="dependfile:glob-*.mk *.d"
@@ -40,7 +40,7 @@ _rgbasm_completions() {
 		[Q]="q-precision:unk"
 		[r]="recursion-depth:unk"
 		[s]="state:unk"
-		[W]="warning:warning"
+		[v]="verbose:normal"
 		[X]="max-errors:unk"
 	)
 	# Parse command-line up to current word

contrib/bash_compl/_rgbfix.bash

@@ -7,15 +7,15 @@ _rgbfix_completions() {
 	# Empty long opt = it doesn't exit
 	# See the `state` variable below for info about `state_after`
 	declare -A opts=(
-		[V]="version:normal"
 		[h]="help:normal"
-		[j]="non-japanese:normal"
+		[V]="version:normal"
-		[s]="sgb-compatible:normal"
+		[W]="warning:warning"
-		[v]="validate:normal"
+		[w]=":normal"
 		[C]="color-only:normal"
 		[c]="color-compatible:normal"
 		[f]="fix-spec:fix-spec"
 		[i]="game-id:unk"
+		[j]="non-japanese:normal"
 		[k]="new-licensee:unk"
 		[L]="custom-logo:glob-*.1bpp"
 		[l]="old-licensee:unk"
@@ -24,7 +24,9 @@ _rgbfix_completions() {
 		[o]="output:glob-*.gb *.gbc *.sgb"
 		[p]="pad-value:unk"
 		[r]="ram-size:unk"
+		[s]="sgb-compatible:normal"
 		[t]="title:unk"
+		[v]="validate:normal"
 	)
 	# Parse command-line up to current word
 	local opt_ena=true
@@ -140,6 +142,16 @@ _rgbfix_completions() {
 	case "$state" in
 		unk) # Return with no replies: no idea what to complete!
 			;;
+		warning)
+			mapfile -t COMPREPLY < <(compgen -W "
+				mbc
+				overwrite
+				sgb
+				truncation
+				all
+				everything
+				error" -P "${cur_word:0:$optlen}" -- "${cur_word:$optlen}")
+			;;
 		fix-spec)
 			COMPREPLY=( "${cur_word}"{l,h,g,L,H,G} )
 			;;

contrib/bash_compl/_rgbgfx.bash

@@ -7,37 +7,38 @@ _rgbgfx_completions() {
 	# Empty long opt = it doesn't exit
 	# See the `state` variable below for info about `state_after`
 	declare -A opts=(
-		[V]="version:normal"
 		[h]="help:normal"
-		[C]="color-curve:normal"
+		[V]="version:normal"
-		[m]="mirror-tiles:normal"
+		[W]="warning:warning"
-		[O]="group-outputs:normal"
+		[w]=":normal"
-		[u]="unique-tiles:normal"
-		[v]="verbose:normal"
-		[X]="mirror-x:normal"
-		[Y]="mirror-y:normal"
-		[Z]="columns:normal"
-		[a]="attr-map:glob-*.attrmap"
 		[A]="auto-attr-map:normal"
+		[a]="attr-map:glob-*.attrmap"
 		[B]="background-color:unk"
 		[b]="base-tiles:unk"
+		[C]="color-curve:normal"
 		[c]="colors:unk"
 		[d]="depth:unk"
 		[i]="input-tileset:glob-*.2bpp"
 		[L]="slice:unk"
+		[m]="mirror-tiles:normal"
 		[N]="nb-tiles:unk"
 		[n]="nb-palettes:unk"
+		[O]="group-outputs:normal"
 		[o]="output:glob-*.2bpp"
-		[p]="palette:glob-*.pal"
 		[P]="auto-palette:normal"
-		[q]="palette-map:glob-*.palmap"
+		[p]="palette:glob-*.pal"
 		[Q]="auto-palette-map:normal"
+		[q]="palette-map:glob-*.palmap"
 		[r]="reverse:unk"
 		[s]="palette-size:unk"
-		[t]="tilemap:glob-*.tilemap"
 		[T]="auto-tilemap:normal"
-		[W]="warning:warning"
+		[t]="tilemap:glob-*.tilemap"
+		[u]="unique-tiles:normal"
+		[v]="verbose:normal"
+		[X]="mirror-x:normal"
 		[x]="trim-end:unk"
+		[Y]="mirror-y:normal"
+		[Z]="columns:normal"
 	)
 	# Parse command-line up to current word
 	local opt_ena=true

contrib/bash_compl/_rgblink.bash

@@ -7,21 +7,21 @@ _rgblink_completions() {
 	# Empty long opt = it doesn't exit
 	# See the `state` variable below for info about `state_after`
 	declare -A opts=(
-		[V]="version:normal"
 		[h]="help:normal"
-		[d]="dmg:normal"
+		[V]="version:normal"
-		[t]="tiny:normal"
+		[W]="warning:warning"
-		[v]="verbose:normal"
-		[w]="wramx:normal"
-		[x]="nopad:normal"
-		[l]="linkerscript:glob-*"
 		[M]="no-sym-in-map:normal"
+		[d]="dmg:normal"
+		[l]="linkerscript:glob-*"
 		[m]="map:glob-*.map"
 		[n]="sym:glob-*.sym"
 		[O]="overlay:glob-*.gb *.gbc *.sgb"
 		[o]="output:glob-*.gb *.gbc *.sgb"
 		[p]="pad:unk"
-		[W]="warning:warning"
+		[t]="tiny:normal"
+		[v]="verbose:normal"
+		[w]="wramx:normal"
+		[x]="nopad:normal"
 	)
 	# Parse command-line up to current word
 	local opt_ena=true

contrib/zsh_compl/_rgbfix

@@ -34,6 +34,21 @@ _mbc_names() {
 	_describe "MBC name" mbc_names
 }
 
+_rgbfix_warnings() {
+	local warnings=(
+		'error:Turn all warnings into errors'
+
+		'all:Enable most warning messages'
+		'everything:Enable literally everything'
+
+		'mbc:Warn about issues with MBC specs'
+		'overwrite:Warn when overwriting non-zero bytes'
+		'sgb:Warn when SGB flag conflicts with old licensee code'
+		'truncation:Warn when values are truncated to fit'
+	)
+	_describe warning warnings
+}
+
 local args=(
 	# Arguments are listed here in the same order as in the manual, except for the version and help
 	'(- : * options)'{-V,--version}'[Print version number and exit]'
@@ -45,6 +60,7 @@ local args=(
 	'(-O --overwrite)'{-O,--overwrite}'[Allow overwriting non-zero bytes]'
 	'(-s --sgb-compatible)'{-s,--sgb-compatible}'[Set the SGB flag]'
 	'(-f --fix-spec -v --validate)'{-v,--validate}'[Shorthand for -f lhg]'
+	-w'[Disable all warnings]'
 
 	'(-f --fix-spec -v --validate)'{-f,--fix-spec}'+[Fix or trash some header values]:fix spec:'
 	'(-i --game-id)'{-i,--game-id}'+[Set game ID string]:4-char game ID:'
@@ -57,6 +73,7 @@ local args=(
 	'(-p --pad-value)'{-p,--pad-value}'+[Pad to next valid size using this byte as padding]:padding byte:'
 	'(-r --ram-size)'{-r,--ram-size}'+[Set RAM size]:ram size byte:'
 	'(-t --title)'{-t,--title}'+[Set title string]:11-char title string:'
+	'(-W --warning)'{-W,--warning}'+[Toggle warning flags]:warning flag:_rgbfix_warnings'
 
 	'*'":ROM files:_files -g '*.{gb,sgb,gbc}'"
 )

contrib/zsh_compl/_rgbgfx

@@ -35,7 +35,7 @@ local args=(
 	'(-q --palette-map -Q --auto-palette-map)'{-Q,--auto-palette-map}'[Shortcut for -p <file>.palmap]'
 	'(-t --tilemap -T --auto-tilemap)'{-T,--auto-tilemap}'[Shortcut for -t <file>.tilemap]'
 	'(-u --unique-tiles)'{-u,--unique-tiles}'[Eliminate redundant tiles]'
-	{-v,--verbose}'[Enable verbose output]'
+	'(-v --verbose)'{-v,--verbose}'[Enable verbose output]'
 	-w'[Disable all warnings]'
 	'(-X --mirror-x)'{-X,--mirror-x}'[Eliminate horizontally mirrored tiles from output]'
 	'(-Y --mirror-y)'{-Y,--mirror-y}'[Eliminate vertically mirrored tiles from output]'

include/fix/mbc.hpp

@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: MIT
+
+#ifndef RGBDS_FIX_MBC_HPP
+#define RGBDS_FIX_MBC_HPP
+
+#include <stdint.h>
+#include <stdio.h>
+
+constexpr uint16_t UNSPECIFIED = 0x200;
+static_assert(UNSPECIFIED > 0xFF, "UNSPECIFIED should not be in byte range!");
+
+enum MbcType {
+	ROM = 0x00,
+	ROM_RAM = 0x08,
+	ROM_RAM_BATTERY = 0x09,
+
+	MBC1 = 0x01,
+	MBC1_RAM = 0x02,
+	MBC1_RAM_BATTERY = 0x03,
+
+	MBC2 = 0x05,
+	MBC2_BATTERY = 0x06,
+
+	MMM01 = 0x0B,
+	MMM01_RAM = 0x0C,
+	MMM01_RAM_BATTERY = 0x0D,
+
+	MBC3 = 0x11,
+	MBC3_TIMER_BATTERY = 0x0F,
+	MBC3_TIMER_RAM_BATTERY = 0x10,
+	MBC3_RAM = 0x12,
+	MBC3_RAM_BATTERY = 0x13,
+
+	MBC5 = 0x19,
+	MBC5_RAM = 0x1A,
+	MBC5_RAM_BATTERY = 0x1B,
+	MBC5_RUMBLE = 0x1C,
+	MBC5_RUMBLE_RAM = 0x1D,
+	MBC5_RUMBLE_RAM_BATTERY = 0x1E,
+
+	MBC6 = 0x20,
+
+	MBC7_SENSOR_RUMBLE_RAM_BATTERY = 0x22,
+
+	POCKET_CAMERA = 0xFC,
+
+	BANDAI_TAMA5 = 0xFD,
+
+	HUC3 = 0xFE,
+
+	HUC1_RAM_BATTERY = 0xFF,
+
+	// "Extended" values (still valid, but not directly actionable)
+
+	// A high byte of 0x01 means TPP1, the low byte is the requested features
+	// This does not include SRAM, which is instead implied by a non-zero SRAM size
+	// Note: Multiple rumble speeds imply rumble
+	TPP1 = 0x100,
+	TPP1_RUMBLE = 0x101,
+	TPP1_MULTIRUMBLE = 0x102, // Should not be possible
+	TPP1_MULTIRUMBLE_RUMBLE = 0x103,
+	TPP1_TIMER = 0x104,
+	TPP1_TIMER_RUMBLE = 0x105,
+	TPP1_TIMER_MULTIRUMBLE = 0x106, // Should not be possible
+	TPP1_TIMER_MULTIRUMBLE_RUMBLE = 0x107,
+	TPP1_BATTERY = 0x108,
+	TPP1_BATTERY_RUMBLE = 0x109,
+	TPP1_BATTERY_MULTIRUMBLE = 0x10A, // Should not be possible
+	TPP1_BATTERY_MULTIRUMBLE_RUMBLE = 0x10B,
+	TPP1_BATTERY_TIMER = 0x10C,
+	TPP1_BATTERY_TIMER_RUMBLE = 0x10D,
+	TPP1_BATTERY_TIMER_MULTIRUMBLE = 0x10E, // Should not be possible
+	TPP1_BATTERY_TIMER_MULTIRUMBLE_RUMBLE = 0x10F,
+
+	// Error values
+	MBC_NONE = UNSPECIFIED, // No MBC specified, do not act on it
+};
+
+void mbc_PrintAcceptedNames(FILE *file);
+
+bool mbc_HasRAM(MbcType type);
+char const *mbc_Name(MbcType type);
+MbcType mbc_ParseName(char const *name, uint8_t &tpp1Major, uint8_t &tpp1Minor);
+
+#endif // RGBDS_FIX_MBC_HPP

include/fix/warning.hpp

@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: MIT
+
+#ifndef RGBDS_FIX_WARNING_HPP
+#define RGBDS_FIX_WARNING_HPP
+
+#include "diagnostics.hpp"
+
+enum WarningLevel {
+	LEVEL_DEFAULT,    // Warnings that are enabled by default
+	LEVEL_ALL,        // Warnings that probably indicate an error
+	LEVEL_EVERYTHING, // Literally every warning
+};
+
+enum WarningID {
+	WARNING_MBC,        // Issues with MBC specs
+	WARNING_OVERWRITE,  // Overwriting non-zero bytes
+	WARNING_SGB,        // SGB flag conflicts with old licensee code
+	WARNING_TRUNCATION, // Truncating values to fit
+
+	NB_PLAIN_WARNINGS,
+
+	NB_WARNINGS = NB_PLAIN_WARNINGS,
+};
+
+extern Diagnostics<WarningLevel, WarningID> warnings;
+
+// Warns the user about problems that don't prevent fixing the ROM
+[[gnu::format(printf, 2, 3)]]
+void warning(WarningID id, char const *fmt, ...);
+
+// Prints an error, and increments the error count
+[[gnu::format(printf, 1, 2)]]
+void error(char const *fmt, ...);
+
+// Prints a fatal error and exits
+[[gnu::format(printf, 1, 2)]]
+void fatal(char const *fmt, ...);
+
+void resetErrors();
+uint32_t checkErrors(char const *filename);
+
+#endif // RGBDS_FIX_WARNING_HPP

man/rgbfix.1

@@ -8,7 +8,7 @@
 .Nd Game Boy header utility and checksum fixer
 .Sh SYNOPSIS
 .Nm
-.Op Fl hjOsVv
+.Op Fl hjOsVvw
 .Op Fl C | c
 .Op Fl f Ar fix_spec
 .Op Fl i Ar game_id
@@ -21,6 +21,7 @@
 .Op Fl p Ar pad_value
 .Op Fl r Ar ram_size
 .Op Fl t Ar title_str
+.Op Fl W Ar warning
 .Op Ar
 .Sh DESCRIPTION
 The
@@ -98,7 +99,7 @@ Print help text for the program and exit.
 Set the game ID string
 .Pq Ad 0x13F Ns \(en Ns Ad 0x142
 to a given string.
-If it's longer than 4 chars, it will be truncated, and a warning emitted.
+If it's longer than 4 characters, it will be truncated.
 .It Fl j , Fl \-non-japanese
 Set the non-Japanese region flag
 .Pq Ad 0x14A
@@ -107,7 +108,7 @@ to 0x01.
 Set the new licensee string
 .Pq Ad 0x144 Ns \(en Ns Ad 0x145
 to a given string.
-If it's longer than 2 chars, it will be truncated, and a warning emitted.
+If it's longer than 2 characters, it will be truncated.
 .It Fl L Ar logo_file , Fl \-logo Ar logo_file
 Specify a logo file to use instead of the official Nintendo logo.
 The file must be 48 bytes of 1bpp tile data; the source image should be 48 pixels wide and 8 pixels tall.
@@ -124,6 +125,8 @@ to a given value from 0 to 0xFF.
 This value may also be an MBC name.
 The list of accepted names can be obtained by passing
 .Ql Cm help
+or
+.Ql Cm list
 as the argument.
 Any amount of whitespace (space and tabs) is allowed around plus signs, and the order of "components" is free, as long as the MBC name is first.
 There are special considerations to take for the TPP1 mapper; see the
@@ -134,7 +137,8 @@ Set the ROM version
 .Pq Ad 0x14C
 to a given value from 0 to 0xFF.
 .It Fl O , Fl \-overwrite
-Allow overwriting different non-zero bytes in the header without a warning being emitted.
+Alias for
+.Fl Wno-overwrite .
 .It Fl o Ar out_file , Fl \-output Ar out_file
 Write the modified ROM image to the given file, or '-' to write to standard output.
 If not specified, the input files are modified in-place, or written to standard output if read from standard input.
@@ -154,15 +158,14 @@ to a given value from 0 to 0xFF.
 Set the SGB flag
 .Pq Ad 0x146
 to 0x03.
-This flag will be ignored by the SGB unless the old licensee code is 0x33!
+This flag will be ignored by the SGB unless the old licensee code
-If this is given as well as
+.Pq Fl -l
-.Fl l ,
+is 0x33!
-but is not set to 0x33, a warning will be printed.
 .It Fl t Ar title , Fl \-title Ar title
 Set the title string
 .Pq Ad 0x134 Ns \(en Ns Ad 0x143
 to a given string.
-If the title is longer than the max length, it will be truncated, and a warning emitted.
+If the title is longer than the maximum length, it will be truncated.
 The max length is 11 characters if the game ID
 .Pq Fl i
 is specified, 15 characters if the CGB flag
@@ -175,6 +178,77 @@ Print the version of the program and exit.
 .It Fl v , Fl \-validate
 Equivalent to
 .Fl f Cm lhg .
+.It Fl W Ar warning , Fl \-warning Ar warning
+Set warning flag
+.Ar warning .
+A warning message will be printed if
+.Ar warning
+is an unknown warning flag.
+See the
+.Sx DIAGNOSTICS
+section for a list of warnings.
+.It Fl w
+Disable all warning output, even when turned into errors.
+.El
+.Sh DIAGNOSTICS
+Warnings are diagnostic messages that indicate possibly erroneous behavior that does not necessarily compromise the header-fixing process.
+The following options alter the way warnings are processed.
+.Bl -tag -width Ds
+.It Fl Werror
+Make all warnings into errors.
+This can be negated as
+.Fl Wno-error
+to prevent turning all warnings into errors.
+.It Fl Werror=
+Make the specified warning or meta warning into an error.
+A warning's name is appended
+.Pq example: Fl Werror=overwrite ,
+and this warning is implicitly enabled and turned into an error.
+This can be negated as
+.Fl Wno-error=
+to prevent turning a specified warning into an error, even if
+.Fl Werror
+is in effect.
+.El
+.Pp
+The following warnings are
+.Dq meta
+warnings, that enable a collection of other warnings.
+If a specific warning is toggled via a meta flag and a specific one, the more specific one takes priority.
+The position on the command-line acts as a tie breaker, the last one taking effect.
+.Bl -tag -width Ds
+.It Fl Wall
+This enables warnings that are likely to indicate an error or undesired behavior, and that can easily be fixed.
+.It Fl Weverything
+Enables literally every warning.
+.El
+.Pp
+The following warnings are actual warning flags; with each description, the corresponding warning flag is included.
+Note that each of these flag also has a negation (for example,
+.Fl Wtruncation
+enables the warning that
+.Fl Wno-truncation
+disables; and
+.Fl Wall
+enables every warning that
+.Fl Wno-all
+disables).
+Only the non-default flag is listed here.
+Ignoring the
+.Dq no-
+prefix, entries are listed alphabetically.
+.Bl -tag -width Ds
+.It Fl Wno-mbc
+Warn when there are inconsistencies with or caveats about the specified MBC type.
+.It Fl Wno-overwrite
+Warn when overwriting different non-zero bytes in the header.
+.It Fl Wno-sgb
+Warn when the SGB flag
+.Pq Fl s
+conflicts with the old licensee code
+.Pq Fl l .
+.It Fl Wno-truncation
+Warn when truncating values to fit the available space.
 .El
 .Sh EXAMPLES
 Most values in the ROM header do not matter to the actual console, and most are seldom useful anyway.
@@ -228,7 +302,7 @@ Therefore,
 .Nm
 will ignore the
 .Ql RAM
-feature on a TPP1 mapper with a warning.
+feature on a TPP1 mapper.
 .Ss Special considerations
 TPP1 overwrites the byte at
 .Ad 0x14A ,

src/CMakeLists.txt

@@ -72,6 +72,8 @@ set(rgblink_src
 
 set(rgbfix_src
     "fix/main.cpp"
+    "fix/mbc.cpp"
+    "fix/warning.cpp"
     )
 
 set(rgbgfx_src

src/fix/mbc.cpp

@@ -0,0 +1,611 @@
+#include "fix/mbc.hpp"
+
+#include <stdlib.h>
+
+#include "helpers.hpp"  // unreachable_
+#include "platform.hpp" // strcasecmp
+
+#include "fix/warning.hpp"
+
+[[gnu::format(printf, 1, 2), noreturn]]
+static void fatalWithMBCNames(char const *fmt, ...) {
+	va_list ap;
+	fputs("FATAL: ", stderr);
+	va_start(ap, fmt);
+	vfprintf(stderr, fmt, ap);
+	va_end(ap);
+	putc('\n', stderr);
+
+	mbc_PrintAcceptedNames(stderr);
+	exit(1);
+}
+
+void mbc_PrintAcceptedNames(FILE *file) {
+	fputs("Accepted MBC names:\n", file);
+	fputs("\tROM ($00) [aka ROM_ONLY]\n", file);
+	fputs("\tMBC1 ($01), MBC1+RAM ($02), MBC1+RAM+BATTERY ($03)\n", file);
+	fputs("\tMBC2 ($05), MBC2+BATTERY ($06)\n", file);
+	fputs("\tROM+RAM ($08) [deprecated], ROM+RAM+BATTERY ($09) [deprecated]\n", file);
+	fputs("\tMMM01 ($0B), MMM01+RAM ($0C), MMM01+RAM+BATTERY ($0D)\n", file);
+	fputs("\tMBC3+TIMER+BATTERY ($0F), MBC3+TIMER+RAM+BATTERY ($10)\n", file);
+	fputs("\tMBC3 ($11), MBC3+RAM ($12), MBC3+RAM+BATTERY ($13)\n", file);
+	fputs("\tMBC5 ($19), MBC5+RAM ($1A), MBC5+RAM+BATTERY ($1B)\n", file);
+	fputs("\tMBC5+RUMBLE ($1C), MBC5+RUMBLE+RAM ($1D), MBC5+RUMBLE+RAM+BATTERY ($1E)\n", file);
+	fputs("\tMBC6 ($20)\n", file);
+	fputs("\tMBC7+SENSOR+RUMBLE+RAM+BATTERY ($22)\n", file);
+	fputs("\tPOCKET_CAMERA ($FC)\n", file);
+	fputs("\tBANDAI_TAMA5 ($FD) [aka TAMA5]\n", file);
+	fputs("\tHUC3 ($FE)\n", file);
+	fputs("\tHUC1+RAM+BATTERY ($FF)\n", file);
+
+	fputs("\n\tTPP1_1.0, TPP1_1.0+RUMBLE, TPP1_1.0+MULTIRUMBLE, TPP1_1.0+TIMER,\n", file);
+	fputs("\tTPP1_1.0+TIMER+RUMBLE, TPP1_1.0+TIMER+MULTIRUMBLE, TPP1_1.0+BATTERY,\n", file);
+	fputs("\tTPP1_1.0+BATTERY+RUMBLE, TPP1_1.0+BATTERY+MULTIRUMBLE,\n", file);
+	fputs("\tTPP1_1.0+BATTERY+TIMER, TPP1_1.0+BATTERY+TIMER+RUMBLE,\n", file);
+	fputs("\tTPP1_1.0+BATTERY+TIMER+MULTIRUMBLE\n", file);
+}
+
+bool mbc_HasRAM(MbcType type) {
+	switch (type) {
+	case ROM_RAM:
+	case ROM_RAM_BATTERY:
+	case MBC1_RAM:
+	case MBC1_RAM_BATTERY:
+	case MMM01_RAM:
+	case MMM01_RAM_BATTERY:
+	case MBC3_TIMER_RAM_BATTERY:
+	case MBC3_RAM:
+	case MBC3_RAM_BATTERY:
+	case MBC5_RAM:
+	case MBC5_RAM_BATTERY:
+	case MBC5_RUMBLE_RAM:
+	case MBC5_RUMBLE_RAM_BATTERY:
+	case MBC6: // "Net de Get - Minigame @ 100" has RAM size 3 (32 KiB)
+	case MBC7_SENSOR_RUMBLE_RAM_BATTERY:
+	case POCKET_CAMERA:
+	case HUC3:
+	case HUC1_RAM_BATTERY:
+		return true;
+
+	case ROM:
+	case MBC1:
+	case MBC2: // Technically has RAM, but not marked as such
+	case MBC2_BATTERY:
+	case MMM01:
+	case MBC3:
+	case MBC3_TIMER_BATTERY:
+	case MBC5:
+	case MBC5_RUMBLE:
+	case BANDAI_TAMA5: // "Game de Hakken!! Tamagotchi - Osutchi to Mesutchi" has RAM size 0
+	case MBC_NONE:
+		return false;
+
+	// TPP1 may or may not have RAM, don't call this function for it
+	case TPP1:
+	case TPP1_RUMBLE:
+	case TPP1_MULTIRUMBLE:
+	case TPP1_MULTIRUMBLE_RUMBLE:
+	case TPP1_TIMER:
+	case TPP1_TIMER_RUMBLE:
+	case TPP1_TIMER_MULTIRUMBLE:
+	case TPP1_TIMER_MULTIRUMBLE_RUMBLE:
+	case TPP1_BATTERY:
+	case TPP1_BATTERY_RUMBLE:
+	case TPP1_BATTERY_MULTIRUMBLE:
+	case TPP1_BATTERY_MULTIRUMBLE_RUMBLE:
+	case TPP1_BATTERY_TIMER:
+	case TPP1_BATTERY_TIMER_RUMBLE:
+	case TPP1_BATTERY_TIMER_MULTIRUMBLE:
+	case TPP1_BATTERY_TIMER_MULTIRUMBLE_RUMBLE:
+		break;
+	}
+
+	unreachable_(); // LCOV_EXCL_LINE
+}
+
+char const *mbc_Name(MbcType type) {
+	switch (type) {
+	case ROM:
+		return "ROM";
+	case ROM_RAM:
+		return "ROM+RAM";
+	case ROM_RAM_BATTERY:
+		return "ROM+RAM+BATTERY";
+	case MBC1:
+		return "MBC1";
+	case MBC1_RAM:
+		return "MBC1+RAM";
+	case MBC1_RAM_BATTERY:
+		return "MBC1+RAM+BATTERY";
+	case MBC2:
+		return "MBC2";
+	case MBC2_BATTERY:
+		return "MBC2+BATTERY";
+	case MMM01:
+		return "MMM01";
+	case MMM01_RAM:
+		return "MMM01+RAM";
+	case MMM01_RAM_BATTERY:
+		return "MMM01+RAM+BATTERY";
+	case MBC3:
+		return "MBC3";
+	case MBC3_TIMER_BATTERY:
+		return "MBC3+TIMER+BATTERY";
+	case MBC3_TIMER_RAM_BATTERY:
+		return "MBC3+TIMER+RAM+BATTERY";
+	case MBC3_RAM:
+		return "MBC3+RAM";
+	case MBC3_RAM_BATTERY:
+		return "MBC3+RAM+BATTERY";
+	case MBC5:
+		return "MBC5";
+	case MBC5_RAM:
+		return "MBC5+RAM";
+	case MBC5_RAM_BATTERY:
+		return "MBC5+RAM+BATTERY";
+	case MBC5_RUMBLE:
+		return "MBC5+RUMBLE";
+	case MBC5_RUMBLE_RAM:
+		return "MBC5+RUMBLE+RAM";
+	case MBC5_RUMBLE_RAM_BATTERY:
+		return "MBC5+RUMBLE+RAM+BATTERY";
+	case MBC6:
+		return "MBC6";
+	case MBC7_SENSOR_RUMBLE_RAM_BATTERY:
+		return "MBC7+SENSOR+RUMBLE+RAM+BATTERY";
+	case POCKET_CAMERA:
+		return "POCKET CAMERA";
+	case BANDAI_TAMA5:
+		return "BANDAI TAMA5";
+	case HUC3:
+		return "HUC3";
+	case HUC1_RAM_BATTERY:
+		return "HUC1+RAM+BATTERY";
+	case TPP1:
+		return "TPP1";
+	case TPP1_RUMBLE:
+		return "TPP1+RUMBLE";
+	case TPP1_MULTIRUMBLE:
+	case TPP1_MULTIRUMBLE_RUMBLE:
+		return "TPP1+MULTIRUMBLE";
+	case TPP1_TIMER:
+		return "TPP1+TIMER";
+	case TPP1_TIMER_RUMBLE:
+		return "TPP1+TIMER+RUMBLE";
+	case TPP1_TIMER_MULTIRUMBLE:
+	case TPP1_TIMER_MULTIRUMBLE_RUMBLE:
+		return "TPP1+TIMER+MULTIRUMBLE";
+	case TPP1_BATTERY:
+		return "TPP1+BATTERY";
+	case TPP1_BATTERY_RUMBLE:
+		return "TPP1+BATTERY+RUMBLE";
+	case TPP1_BATTERY_MULTIRUMBLE:
+	case TPP1_BATTERY_MULTIRUMBLE_RUMBLE:
+		return "TPP1+BATTERY+MULTIRUMBLE";
+	case TPP1_BATTERY_TIMER:
+		return "TPP1+BATTERY+TIMER";
+	case TPP1_BATTERY_TIMER_RUMBLE:
+		return "TPP1+BATTERY+TIMER+RUMBLE";
+	case TPP1_BATTERY_TIMER_MULTIRUMBLE:
+	case TPP1_BATTERY_TIMER_MULTIRUMBLE_RUMBLE:
+		return "TPP1+BATTERY+TIMER+MULTIRUMBLE";
+	case MBC_NONE:
+		break;
+	}
+
+	unreachable_(); // LCOV_EXCL_LINE
+}
+
+static void skipWhitespace(char const *&ptr) {
+	while (*ptr == ' ' || *ptr == '\t') {
+		++ptr;
+	}
+}
+
+static void skipMBCSpace(char const *&ptr) {
+	while (*ptr == ' ' || *ptr == '\t' || *ptr == '_') {
+		++ptr;
+	}
+}
+
+static char normalizeMBCChar(char c) {
+	if (c >= 'a' && c <= 'z') { // Uppercase for comparison with `mbc_Name`s
+		c = c - 'a' + 'A';
+	} else if (c == '_') { // Treat underscores as spaces
+		c = ' ';
+	}
+	return c;
+}
+
+static bool readMBCSlice(char const *&name, char const *expected) {
+	while (*expected) {
+		// If `name` is too short, the character will be '\0' and this will return `false`
+		if (normalizeMBCChar(*name++) != *expected++) {
+			return false;
+		}
+	}
+	return true;
+}
+
+[[noreturn]]
+static void fatalUnknownMBC(char const *fullName) {
+	fatalWithMBCNames("Unknown MBC \"%s\"", fullName);
+}
+
+[[noreturn]]
+static void fatalWrongMBCFeatures(char const *fullName) {
+	fatalWithMBCNames("Features incompatible with MBC (\"%s\")", fullName);
+}
+
+MbcType mbc_ParseName(char const *name, uint8_t &tpp1Major, uint8_t &tpp1Minor) {
+	char const *fullName = name;
+
+	if (!strcasecmp(name, "help") || !strcasecmp(name, "list")) {
+		mbc_PrintAcceptedNames(stdout);
+		exit(0);
+	}
+
+	if ((name[0] >= '0' && name[0] <= '9') || name[0] == '$') {
+		int base = 0;
+
+		if (name[0] == '$') {
+			++name;
+			base = 16;
+		}
+		// Parse number, and return it as-is (unless it's too large)
+		char *endptr;
+		unsigned long mbc = strtoul(name, &endptr, base);
+
+		if (*endptr) {
+			fatalUnknownMBC(fullName);
+		}
+		if (mbc > 0xFF) {
+			fatal("Specified MBC ID out of range 0-255: %s", fullName);
+		}
+		return static_cast<MbcType>(mbc);
+	}
+
+	// Begin by reading the MBC type:
+	uint16_t mbc;
+	char const *ptr = name;
+
+	skipWhitespace(ptr); // Trim off leading whitespace
+
+#define tryReadSlice(expected) \
+	do { \
+		if (!readMBCSlice(ptr, expected)) { \
+			fatalUnknownMBC(fullName); \
+		} \
+	} while (0)
+
+	switch (*ptr++) {
+	case 'R': // ROM / ROM_ONLY
+	case 'r':
+		tryReadSlice("OM");
+		// Handle optional " ONLY"
+		skipMBCSpace(ptr);
+		if (*ptr == 'O' || *ptr == 'o') {
+			++ptr;
+			tryReadSlice("NLY");
+		}
+		mbc = ROM;
+		break;
+
+	case 'M': // MBC{1, 2, 3, 5, 6, 7} / MMM01
+	case 'm':
+		switch (*ptr++) {
+		case 'B':
+		case 'b':
+			switch (*ptr++) {
+			case 'C':
+			case 'c':
+				break;
+			default:
+				fatalUnknownMBC(fullName);
+			}
+			switch (*ptr++) {
+			case '1':
+				mbc = MBC1;
+				break;
+			case '2':
+				mbc = MBC2;
+				break;
+			case '3':
+				mbc = MBC3;
+				break;
+			case '5':
+				mbc = MBC5;
+				break;
+			case '6':
+				mbc = MBC6;
+				break;
+			case '7':
+				mbc = MBC7_SENSOR_RUMBLE_RAM_BATTERY;
+				break;
+			default:
+				fatalUnknownMBC(fullName);
+			}
+			break;
+		case 'M':
+		case 'm':
+			tryReadSlice("M01");
+			mbc = MMM01;
+			break;
+		default:
+			fatalUnknownMBC(fullName);
+		}
+		break;
+
+	case 'P': // POCKET_CAMERA
+	case 'p':
+		tryReadSlice("OCKET CAMERA");
+		mbc = POCKET_CAMERA;
+		break;
+
+	case 'B': // BANDAI_TAMA5
+	case 'b':
+		tryReadSlice("ANDAI TAMA5");
+		mbc = BANDAI_TAMA5;
+		break;
+
+	case 'T': // TAMA5 / TPP1
+	case 't':
+		switch (*ptr++) {
+		case 'A':
+			tryReadSlice("MA5");
+			mbc = BANDAI_TAMA5;
+			break;
+		case 'P': {
+			tryReadSlice("P1");
+			// Parse version
+			skipMBCSpace(ptr);
+			// Major
+			char *endptr;
+			unsigned long val = strtoul(ptr, &endptr, 10);
+
+			if (endptr == ptr) {
+				fatal("Failed to parse TPP1 major revision number");
+			}
+			ptr = endptr;
+			if (val != 1) {
+				fatal("RGBFIX only supports TPP1 version 1.0");
+			}
+			tpp1Major = val;
+			tryReadSlice(".");
+			// Minor
+			val = strtoul(ptr, &endptr, 10);
+			if (endptr == ptr) {
+				fatal("Failed to parse TPP1 minor revision number");
+			}
+			ptr = endptr;
+			if (val > 0xFF) {
+				fatal("TPP1 minor revision number must be 8-bit");
+			}
+			tpp1Minor = val;
+			mbc = TPP1;
+			break;
+		}
+		default:
+			fatalUnknownMBC(fullName);
+		}
+		break;
+
+	case 'H': // HuC{1, 3}
+	case 'h':
+		tryReadSlice("UC");
+		switch (*ptr++) {
+		case '1':
+			mbc = HUC1_RAM_BATTERY;
+			break;
+		case '3':
+			mbc = HUC3;
+			break;
+		default:
+			fatalUnknownMBC(fullName);
+		}
+		break;
+
+	default:
+		fatalUnknownMBC(fullName);
+	}
+
+	// Read "additional features"
+	uint8_t features = 0;
+	// clang-format off: vertically align values
+	static constexpr uint8_t RAM         = 1 << 7;
+	static constexpr uint8_t BATTERY     = 1 << 6;
+	static constexpr uint8_t TIMER       = 1 << 5;
+	static constexpr uint8_t RUMBLE      = 1 << 4;
+	static constexpr uint8_t SENSOR      = 1 << 3;
+	static constexpr uint8_t MULTIRUMBLE = 1 << 2;
+	// clang-format on
+
+	for (;;) {
+		skipWhitespace(ptr); // Trim off trailing whitespace
+
+		// If done, start processing "features"
+		if (!*ptr) {
+			break;
+		}
+		// We expect a '+' at this point
+		skipMBCSpace(ptr);
+		if (*ptr++ != '+') {
+			fatalUnknownMBC(fullName);
+		}
+		skipMBCSpace(ptr);
+
+		switch (*ptr++) {
+		case 'B': // BATTERY
+		case 'b':
+			tryReadSlice("ATTERY");
+			features |= BATTERY;
+			break;
+
+		case 'M':
+		case 'm':
+			tryReadSlice("ULTIRUMBLE");
+			features |= MULTIRUMBLE;
+			break;
+
+		case 'R': // RAM or RUMBLE
+		case 'r':
+			switch (*ptr++) {
+			case 'U':
+			case 'u':
+				tryReadSlice("MBLE");
+				features |= RUMBLE;
+				break;
+			case 'A':
+			case 'a':
+				tryReadSlice("M");
+				features |= RAM;
+				break;
+			default:
+				fatalUnknownMBC(fullName);
+			}
+			break;
+
+		case 'S': // SENSOR
+		case 's':
+			tryReadSlice("ENSOR");
+			features |= SENSOR;
+			break;
+
+		case 'T': // TIMER
+		case 't':
+			tryReadSlice("IMER");
+			features |= TIMER;
+			break;
+
+		default:
+			fatalUnknownMBC(fullName);
+		}
+	}
+#undef tryReadSlice
+
+	switch (mbc) {
+	case ROM:
+		if (!features) {
+			break;
+		}
+		mbc = ROM_RAM - 1;
+		static_assert(ROM_RAM + 1 == ROM_RAM_BATTERY, "Enum sanity check failed!");
+		static_assert(MBC1 + 1 == MBC1_RAM, "Enum sanity check failed!");
+		static_assert(MBC1 + 2 == MBC1_RAM_BATTERY, "Enum sanity check failed!");
+		static_assert(MMM01 + 1 == MMM01_RAM, "Enum sanity check failed!");
+		static_assert(MMM01 + 2 == MMM01_RAM_BATTERY, "Enum sanity check failed!");
+		[[fallthrough]];
+	case MBC1:
+	case MMM01:
+		if (features == RAM) {
+			++mbc;
+		} else if (features == (RAM | BATTERY)) {
+			mbc += 2;
+		} else if (features) {
+			fatalWrongMBCFeatures(fullName);
+		}
+		break;
+
+	case MBC2:
+		if (features == BATTERY) {
+			mbc = MBC2_BATTERY;
+		} else if (features) {
+			fatalWrongMBCFeatures(fullName);
+		}
+		break;
+
+	case MBC3:
+		// Handle timer, which also requires battery
+		if (features & TIMER) {
+			if (!(features & BATTERY)) {
+				warning(WARNING_MBC, "MBC3+TIMER implies BATTERY");
+			}
+			features &= ~(TIMER | BATTERY); // Reset those bits
+			mbc = MBC3_TIMER_BATTERY;
+			// RAM is handled below
+		}
+		static_assert(MBC3 + 1 == MBC3_RAM, "Enum sanity check failed!");
+		static_assert(MBC3 + 2 == MBC3_RAM_BATTERY, "Enum sanity check failed!");
+		static_assert(
+		    MBC3_TIMER_BATTERY + 1 == MBC3_TIMER_RAM_BATTERY, "Enum sanity check failed!"
+		);
+		if (features == RAM) {
+			++mbc;
+		} else if (features == (RAM | BATTERY)) {
+			mbc += 2;
+		} else if (features) {
+			fatalWrongMBCFeatures(fullName);
+		}
+		break;
+
+	case MBC5:
+		if (features & RUMBLE) {
+			features &= ~RUMBLE;
+			mbc = MBC5_RUMBLE;
+		}
+		static_assert(MBC5 + 1 == MBC5_RAM, "Enum sanity check failed!");
+		static_assert(MBC5 + 2 == MBC5_RAM_BATTERY, "Enum sanity check failed!");
+		static_assert(MBC5_RUMBLE + 1 == MBC5_RUMBLE_RAM, "Enum sanity check failed!");
+		static_assert(MBC5_RUMBLE + 2 == MBC5_RUMBLE_RAM_BATTERY, "Enum sanity check failed!");
+		if (features == RAM) {
+			++mbc;
+		} else if (features == (RAM | BATTERY)) {
+			mbc += 2;
+		} else if (features) {
+			fatalWrongMBCFeatures(fullName);
+		}
+		break;
+
+	case MBC6:
+	case POCKET_CAMERA:
+	case BANDAI_TAMA5:
+	case HUC3:
+		// No extra features accepted
+		if (features) {
+			fatalWrongMBCFeatures(fullName);
+		}
+		break;
+
+	case MBC7_SENSOR_RUMBLE_RAM_BATTERY:
+		if (features != (SENSOR | RUMBLE | RAM | BATTERY)) {
+			fatalWrongMBCFeatures(fullName);
+		}
+		break;
+
+	case HUC1_RAM_BATTERY:
+		if (features != (RAM | BATTERY)) { // HuC1 expects RAM+BATTERY
+			fatalWrongMBCFeatures(fullName);
+		}
+		break;
+
+	case TPP1:
+		if (features & RAM) {
+			warning(WARNING_MBC, "TPP1 requests RAM implicitly if given a non-zero RAM size");
+		}
+		if (features & BATTERY) {
+			mbc |= 0x08;
+		}
+		if (features & TIMER) {
+			mbc |= 0x04;
+		}
+		if (features & MULTIRUMBLE) {
+			mbc |= 0x03; // Also set the rumble flag
+		}
+		if (features & RUMBLE) {
+			mbc |= 0x01;
+		}
+		if (features & SENSOR) {
+			fatalWrongMBCFeatures(fullName);
+		}
+		break;
+	}
+
+	skipWhitespace(ptr); // Trim off trailing whitespace
+
+	// If there is still something past the whitespace, error out
+	if (*ptr) {
+		fatalUnknownMBC(fullName);
+	}
+
+	return static_cast<MbcType>(mbc);
+}

src/fix/warning.cpp

@@ -0,0 +1,93 @@
+#include "fix/warning.hpp"
+
+static uint32_t nbErrors;
+
+// clang-format off: nested initializers
+Diagnostics<WarningLevel, WarningID> warnings = {
+    .metaWarnings = {
+        {"all",        LEVEL_ALL       },
+        {"everything", LEVEL_EVERYTHING},
+    },
+    .warningFlags = {
+        {"mbc",        LEVEL_DEFAULT   },
+        {"overwrite",  LEVEL_DEFAULT   },
+        {"sgb",        LEVEL_DEFAULT   },
+        {"truncation", LEVEL_DEFAULT   },
+    },
+    .paramWarnings = {},
+    .state = DiagnosticsState<WarningID>(),
+};
+// clang-format on
+
+void resetErrors() {
+	nbErrors = 0;
+}
+
+uint32_t checkErrors(char const *filename) {
+	if (nbErrors > 0) {
+		fprintf(
+		    stderr,
+		    "Fixing \"%s\" failed with %u error%s\n",
+		    filename,
+		    nbErrors,
+		    nbErrors == 1 ? "" : "s"
+		);
+	}
+	return nbErrors;
+}
+
+static void incrementErrors() {
+	if (nbErrors != UINT32_MAX) {
+		++nbErrors;
+	}
+}
+
+void error(char const *fmt, ...) {
+	va_list ap;
+	fputs("error: ", stderr);
+	va_start(ap, fmt);
+	vfprintf(stderr, fmt, ap);
+	va_end(ap);
+	putc('\n', stderr);
+
+	incrementErrors();
+}
+
+void fatal(char const *fmt, ...) {
+	va_list ap;
+	fputs("FATAL: ", stderr);
+	va_start(ap, fmt);
+	vfprintf(stderr, fmt, ap);
+	va_end(ap);
+	putc('\n', stderr);
+
+	exit(1);
+}
+
+void warning(WarningID id, char const *fmt, ...) {
+	char const *flag = warnings.warningFlags[id].name;
+	va_list ap;
+
+	switch (warnings.getWarningBehavior(id)) {
+	case WarningBehavior::DISABLED:
+		break;
+
+	case WarningBehavior::ENABLED:
+		fprintf(stderr, "warning: [-W%s]\n    ", flag);
+		va_start(ap, fmt);
+		vfprintf(stderr, fmt, ap);
+		va_end(ap);
+		putc('\n', stderr);
+		break;
+
+	case WarningBehavior::ERROR:
+		fprintf(stderr, "error: [-Werror=%s]\n    ", flag);
+		va_start(ap, fmt);
+		vfprintf(stderr, fmt, ap);
+		va_end(ap);
+		putc('\n', stderr);
+
+		incrementErrors();
+		break;
+	}
+}

test/fix/bad-fix-char.bin

@@ -1,3 +0,0 @@
-‡Ý
à!H0‡;<ýNË—ñãí=ÌÊÏç^{KC}b“Q¤&<14>3ð ñ/]
¿—¹”d<E2809D>ÙZ=ÃÀ¢bÄaËi:öeSŸ>ñ›U@¼Ay<aeš‰¹$vfÅå­ˆ┚ý¬gŒ	VC†ò~>•‘¨þ©†	³s_CÓ›Úç&nWFoKÚšíX`'ñêÁ§ë·blƒ{‡›Œ¿
-‹¯­çÎ8îù†<$:VÍ"ã³™Ú<E284A2>h™ÑD©¤Ð¶ñ­QcµïØÐ㸺Zû§T†ž©ø½¨ºVÛ¼ÝtÅ’ªÏ4ªBèÐdT“«	ÄI<1A>‡
-Ô(/×»ç¿{hÝ:ƒ<>QmÓ$*|
 íVPéó.^"&é<äŒk!š<>é`àù2ÝÝêê
&Bû	lÍ7föèšZe¤yf.˜mƒýØ¯!³Œ<C2B3><C592>óN5„õd†EÁ¦qF×

test/fix/bad-fix-char.err

@@ -1,4 +1 @@
-warning: Ignoring 'm' in fix spec
+FATAL: Invalid character 'm' in fix spec
-warning: Ignoring 'a' in fix spec
-warning: Ignoring 'o' in fix spec
-warning: Overwrote a non-zero byte in the Nintendo logo

test/fix/color.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the CGB flag
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the CGB flag

test/fix/compatible.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the CGB flag
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the CGB flag

test/fix/custom-logo.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the logo
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the logo

test/fix/dollar-hex.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the cartridge type
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the cartridge type

test/fix/empty.err

@@ -1,2 +1,2 @@
-FATAL: "<filename>" too short, expected at least 336 ($150) bytes, got only 0
+error: "<filename>" too short, expected at least 336 ($150) bytes, got only 0
 Fixing "<filename>" failed with 1 error

test/fix/fix-override.err

@@ -1,2 +1,3 @@
 warning: 'l' overriding 'L' in fix spec
-warning: Overwrote a non-zero byte in the Nintendo logo
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the Nintendo logo

test/fix/gameid-trunc.err

@@ -1,2 +1,4 @@
-warning: Truncating game ID "FOUR!" to 4 chars
+warning: [-Wtruncation]
-warning: Overwrote a non-zero byte in the manufacturer code
+    Truncating game ID "FOUR!" to 4 chars
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the manufacturer code

test/fix/gameid.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the manufacturer code
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the manufacturer code

test/fix/global-large.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the global checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the global checksum

test/fix/global-larger.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the global checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the global checksum

test/fix/global-trash.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the global checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the global checksum

test/fix/global.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the global checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the global checksum

test/fix/header-edit.err

@@ -1,2 +1,4 @@
-warning: Overwrote a non-zero byte in the CGB flag
+warning: [-Woverwrite]
-warning: Overwrote a non-zero byte in the header checksum
+    Overwrote a non-zero byte in the CGB flag
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the header checksum

test/fix/header-trash.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the header checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the header checksum

test/fix/header.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the header checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the header checksum

test/fix/incompatible-features.err

@@ -1,4 +1,4 @@
-error: Features incompatible with MBC ("mbc1+multirumble")
+FATAL: Features incompatible with MBC ("mbc1+multirumble")
 Accepted MBC names:
 	ROM ($00) [aka ROM_ONLY]
 	MBC1 ($01), MBC1+RAM ($02), MBC1+RAM+BATTERY ($03)

test/fix/jp.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the destination code
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the destination code

test/fix/logo-trash.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the Nintendo logo
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the Nintendo logo

test/fix/logo.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the Nintendo logo
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the Nintendo logo

test/fix/mbc-rom-ram-battery.err

@@ -1 +1,2 @@
-warning: MBC "ROM+RAM+BATTERY" is under-specified and poorly supported
+warning: [-Wmbc]
+    MBC "ROM+RAM+BATTERY" is under-specified and poorly supported

test/fix/mbc-rom-ram.err

@@ -1 +1,2 @@
-warning: MBC "ROM+RAM" is under-specified and poorly supported
+warning: [-Wmbc]
+    MBC "ROM+RAM" is under-specified and poorly supported

test/fix/mbc.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the cartridge type
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the cartridge type

test/fix/mbcless-ram.err

@@ -1,3 +1,6 @@
-warning: MBC "ROM" has no RAM, but RAM size was set to 2
+warning: [-Wmbc]
-warning: Overwrote a non-zero byte in the cartridge type
+    MBC "ROM" has no RAM, but RAM size was set to 2
-warning: Overwrote a non-zero byte in the RAM size
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the cartridge type
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the RAM size

test/fix/new-lic-trunc.err

@@ -1,2 +1,4 @@
-warning: Truncating new licensee "HOMEBREW" to 2 chars
+warning: [-Wtruncation]
-warning: Overwrote a non-zero byte in the new licensee code
+    Truncating new licensee "HOMEBREW" to 2 chars
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the new licensee code

test/fix/new-lic.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the new licensee code
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the new licensee code

test/fix/noexist.err

@@ -1,2 +1,2 @@
-FATAL: Failed to open "noexist" for reading+writing: No such file or directory
+error: Failed to open "noexist" for reading+writing: No such file or directory
 Fixing "noexist" failed with 1 error

test/fix/old-lic-hex.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the old licensee code
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the old licensee code

test/fix/old-lic.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the old licensee code
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the old licensee code

test/fix/ram.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the RAM size
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the RAM size

test/fix/ramful-mbc-no-ram.err

@@ -1,3 +1,6 @@
-warning: MBC "MBC3+RAM" has RAM, but RAM size was set to 0
+warning: [-Wmbc]
-warning: Overwrote a non-zero byte in the cartridge type
+    MBC "MBC3+RAM" has RAM, but RAM size was set to 0
-warning: Overwrote a non-zero byte in the RAM size
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the cartridge type
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the RAM size

test/fix/ramful-mbc.err

@@ -1,2 +1,4 @@
-warning: Overwrote a non-zero byte in the cartridge type
+warning: [-Woverwrite]
-warning: Overwrote a non-zero byte in the RAM size
+    Overwrote a non-zero byte in the cartridge type
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the RAM size

test/fix/ramless-mbc.err

@@ -1,2 +1,4 @@
-warning: Overwrote a non-zero byte in the cartridge type
+warning: [-Woverwrite]
-warning: Overwrote a non-zero byte in the RAM size
+    Overwrote a non-zero byte in the cartridge type
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the RAM size

test/fix/sgb-licensee.err

@@ -1,3 +1,6 @@
-warning: SGB compatibility enabled, but old licensee is 0x45, not 0x33
+warning: [-Wsgb]
-warning: Overwrote a non-zero byte in the SGB flag
+    SGB compatibility enabled, but old licensee is 0x45, not 0x33
-warning: Overwrote a non-zero byte in the old licensee code
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the SGB flag
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the old licensee code

test/fix/sgb-old-licensee.err

@@ -1,2 +1,4 @@
-warning: Overwrote a non-zero byte in the SGB flag
+warning: [-Woverwrite]
-warning: SGB compatibility enabled, but old licensee was 0xc5, not 0x33
+    Overwrote a non-zero byte in the SGB flag
+warning: [-Wsgb]
+    SGB compatibility enabled, but old licensee was 0xc5, not 0x33

test/fix/sgb.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the SGB flag
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the SGB flag

test/fix/test.sh

@@ -58,31 +58,33 @@ runTest () {
 		fi
 		if [[ -z "$variant" ]]; then
 			cp "$desired_input" out.gb
-			if [[ -n "$(eval "$RGBFIX" $flags out.gb '2>out.err')" ]]; then
+			eval "$RGBFIX" $flags out.gb '>out.out' '2>out.err'
-				echo "${bold}${red}Fixing $1 in-place shouldn't output anything on stdout!${rescolors}${resbold}"
-				our_rc=1
-			fi
 			subst=out.gb
 		elif [[ "$variant" = ' piped' ]]; then
 			# Stop! This is not a Useless Use Of Cat. Using cat instead of
 			# stdin redirection makes the input an unseekable pipe - a scenario
 			# that's harder to deal with.
 			# shellcheck disable=SC2002
-			cat "$desired_input" | eval $RGBFIX "$flags" - '>out.gb' '2>out.err'
+			cat "$desired_input" | eval "$RGBFIX" $flags - '>out.gb' '2>out.err'
 			subst='<stdin>'
 		elif [[ "$variant" = ' output' ]]; then
 			cp "$desired_input" input.gb
-			if [[ -n "$(eval "$RGBFIX" $flags -o out.gb input.gb '2>out.err')" ]]; then
+			eval "$RGBFIX" $flags -o out.gb input.gb '>out.out' '2>out.err'
-				our_rc=1
-			fi
 			subst=input.gb
 		fi
 
+		if [[ -r "$2/$1.out" ]]; then
+			desired_outname="$2/$1.out"
+		else
+			desired_outname=/dev/null
+		fi
 		if [[ -r "$2/$1.err" ]]; then
 			desired_errname="$2/$1.err"
 		else
 			desired_errname=/dev/null
 		fi
+		sed "s/$subst/<filename>/g" out.out | tryDiff "$desired_outname" - "$1.out${variant}"
+		(( our_rc = our_rc || $? ))
 		sed "s/$subst/<filename>/g" out.err | tryDiff "$desired_errname" - "$1.err${variant}"
 		(( our_rc = our_rc || $? ))
 

test/fix/title-color-trunc-rev.err

@@ -1,3 +1,6 @@
-warning: Truncating title "0123456789ABCDEF" to 15 chars
+warning: [-Wtruncation]
-warning: Overwrote a non-zero byte in the title
+    Truncating title "0123456789ABCDEF" to 15 chars
-warning: Overwrote a non-zero byte in the CGB flag
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the title
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the CGB flag

test/fix/title-color-trunc.err

@@ -1,3 +1,6 @@
-warning: Truncating title "0123456789ABCDEF" to 15 chars
+warning: [-Wtruncation]
-warning: Overwrote a non-zero byte in the title
+    Truncating title "0123456789ABCDEF" to 15 chars
-warning: Overwrote a non-zero byte in the CGB flag
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the title
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the CGB flag

test/fix/title-compat-trunc-rev.err

@@ -1,3 +1,6 @@
-warning: Truncating title "0123456789ABCDEF" to 15 chars
+warning: [-Wtruncation]
-warning: Overwrote a non-zero byte in the title
+    Truncating title "0123456789ABCDEF" to 15 chars
-warning: Overwrote a non-zero byte in the CGB flag
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the title
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the CGB flag

test/fix/title-compat-trunc.err

@@ -1,3 +1,6 @@
-warning: Truncating title "0123456789ABCDEF" to 15 chars
+warning: [-Wtruncation]
-warning: Overwrote a non-zero byte in the title
+    Truncating title "0123456789ABCDEF" to 15 chars
-warning: Overwrote a non-zero byte in the CGB flag
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the title
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the CGB flag

test/fix/title-gameid-trunc-rev.err

@@ -1,3 +1,6 @@
-warning: Truncating title "0123456789ABCDEF" to 11 chars
+warning: [-Wtruncation]
-warning: Overwrote a non-zero byte in the title
+    Truncating title "0123456789ABCDEF" to 11 chars
-warning: Overwrote a non-zero byte in the manufacturer code
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the title
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the manufacturer code

test/fix/title-gameid-trunc.err

@@ -1,3 +1,6 @@
-warning: Truncating title "0123456789ABCDEF" to 11 chars
+warning: [-Wtruncation]
-warning: Overwrote a non-zero byte in the title
+    Truncating title "0123456789ABCDEF" to 11 chars
-warning: Overwrote a non-zero byte in the manufacturer code
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the title
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the manufacturer code

test/fix/title-pad.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the title
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the title

test/fix/title-trunc.err

@@ -1,2 +1,4 @@
-warning: Truncating title "0123456789ABCDEFGHIJK" to 16 chars
+warning: [-Wtruncation]
-warning: Overwrote a non-zero byte in the title
+    Truncating title "0123456789ABCDEFGHIJK" to 16 chars
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the title

test/fix/title.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the title
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the title

test/fix/tooshort.err

@@ -1,2 +1,2 @@
-FATAL: "<filename>" too short, expected at least 336 ($150) bytes, got only 20
+error: "<filename>" too short, expected at least 336 ($150) bytes, got only 20
 Fixing "<filename>" failed with 1 error

test/fix/tpp1-bad-major.err

@@ -1 +1 @@
-error: Failed to parse TPP1 major revision number
+FATAL: Failed to parse TPP1 major revision number

test/fix/tpp1-bad-minor.err

@@ -1 +1 @@
-error: Failed to parse TPP1 minor revision number
+FATAL: Failed to parse TPP1 minor revision number

test/fix/tpp1-features.err

@@ -1,5 +1,10 @@
-warning: Overwrote a non-zero byte in the cartridge type
+warning: [-Woverwrite]
-warning: Overwrote a non-zero byte in the TPP1 identification code
+    Overwrote a non-zero byte in the cartridge type
-warning: Overwrote a non-zero byte in the TPP1 revision number
+warning: [-Woverwrite]
-warning: Overwrote a non-zero byte in the RAM size
+    Overwrote a non-zero byte in the TPP1 identification code
-warning: Overwrote a non-zero byte in the TPP1 feature flags
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the TPP1 revision number
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the RAM size
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the TPP1 feature flags

test/fix/tpp1-nonjap.err

@@ -1,5 +1,10 @@
-warning: TPP1 overwrites region flag for its identification code, ignoring `-j`
+warning: [-Wmbc]
-warning: Overwrote a non-zero byte in the cartridge type
+    TPP1 overwrites region flag for its identification code, ignoring `-j`
-warning: Overwrote a non-zero byte in the TPP1 identification code
+warning: [-Woverwrite]
-warning: Overwrote a non-zero byte in the TPP1 revision number
+    Overwrote a non-zero byte in the cartridge type
-warning: Overwrote a non-zero byte in the TPP1 feature flags
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the TPP1 identification code
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the TPP1 revision number
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the TPP1 feature flags

test/fix/tpp1-too-short.err

@@ -1,2 +1,2 @@
-FATAL: "<filename>" too short, expected at least 340 ($154) bytes, got only 339
+error: "<filename>" too short, expected at least 340 ($154) bytes, got only 339
 Fixing "<filename>" failed with 1 error

test/fix/tpp1-unk-major.err

@@ -1 +1 @@
-error: RGBFIX only supports TPP1 version 1.0
+FATAL: RGBFIX only supports TPP1 version 1.0

test/fix/tpp1-unk-minor.err

@@ -1 +1 @@
-error: TPP1 minor revision number must be 8-bit
+FATAL: TPP1 minor revision number must be 8-bit

test/fix/unknown-mbc.err

@@ -1,4 +1,4 @@
-error: Unknown MBC "MBC1337"
+FATAL: Unknown MBC "MBC1337"
 Accepted MBC names:
 	ROM ($00) [aka ROM_ONLY]
 	MBC1 ($01), MBC1+RAM ($02), MBC1+RAM+BATTERY ($03)

test/fix/verify-pad.err

@@ -1,3 +1,6 @@
-warning: Overwrote a non-zero byte in the Nintendo logo
+warning: [-Woverwrite]
-warning: Overwrote a non-zero byte in the header checksum
+    Overwrote a non-zero byte in the Nintendo logo
-warning: Overwrote a non-zero byte in the global checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the header checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the global checksum

test/fix/verify-trash.err

@@ -1,3 +1,6 @@
-warning: Overwrote a non-zero byte in the Nintendo logo
+warning: [-Woverwrite]
-warning: Overwrote a non-zero byte in the header checksum
+    Overwrote a non-zero byte in the Nintendo logo
-warning: Overwrote a non-zero byte in the global checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the header checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the global checksum

test/fix/verify.err

@@ -1,3 +1,6 @@
-warning: Overwrote a non-zero byte in the Nintendo logo
+warning: [-Woverwrite]
-warning: Overwrote a non-zero byte in the header checksum
+    Overwrote a non-zero byte in the Nintendo logo
-warning: Overwrote a non-zero byte in the global checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the header checksum
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the global checksum

test/fix/version.err

@@ -1 +1,2 @@
-warning: Overwrote a non-zero byte in the mask ROM version number
+warning: [-Woverwrite]
+    Overwrote a non-zero byte in the mask ROM version number