From bc8d99d915c68f3c0736749fa1325bac2928adc1 Mon Sep 17 00:00:00 2001 From: John Millikin Date: Sun, 20 Apr 2025 12:17:11 +0900 Subject: [PATCH] Add `-o` / `--output` option to `rgbfix` to write separate output files (#1666) --- contrib/bash_compl/_rgbfix.bash | 1 + contrib/zsh_compl/_rgbfix | 1 + man/rgbfix.1 | 4 ++ src/fix/main.cpp | 65 +++++++++++++++++++++++++++------ test/fix/test.sh | 10 ++++- 5 files changed, 68 insertions(+), 13 deletions(-) diff --git a/contrib/bash_compl/_rgbfix.bash b/contrib/bash_compl/_rgbfix.bash index a8f03bf2..f36c96e9 100755 --- a/contrib/bash_compl/_rgbfix.bash +++ b/contrib/bash_compl/_rgbfix.bash @@ -21,6 +21,7 @@ _rgbfix_completions() { [l]="old-licensee:unk" [m]="mbc-type:mbc" [n]="rom-version:unk" + [o]="output:glob-*.gb *.gbc *.sgb" [p]="pad-value:unk" [r]="ram-size:unk" [t]="title:unk" diff --git a/contrib/zsh_compl/_rgbfix b/contrib/zsh_compl/_rgbfix index c559a3b1..11d157ac 100644 --- a/contrib/zsh_compl/_rgbfix +++ b/contrib/zsh_compl/_rgbfix @@ -53,6 +53,7 @@ local args=( '(-L --logo)'{-L,--logo}'+[Set custom logo]:1bpp image:' '(-m --mbc-type)'{-m,--mbc-type}"+[Set MBC flags]:mbc name:_mbc_names" '(-n --rom-version)'{-n,--rom-version}'+[Set ROM version]:rom version byte:' + '(-o --output)'{-o,--output}"+[Output file]:output file:_files -g '*.{gb,sgb,gbc}'" '(-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:' diff --git a/man/rgbfix.1 b/man/rgbfix.1 index 1f1dc435..cc40402e 100644 --- a/man/rgbfix.1 +++ b/man/rgbfix.1 @@ -17,6 +17,7 @@ .Op Fl l Ar licensee_id .Op Fl m Ar mbc_type .Op Fl n Ar rom_version +.Op Fl o Ar out_file .Op Fl p Ar pad_value .Op Fl r Ar ram_size .Op Fl t Ar title_str @@ -134,6 +135,9 @@ Set the ROM version 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. +.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. .It Fl p Ar pad_value , Fl \-pad-value Ar pad_value Pad the ROM image to a valid size with a given pad value from 0 to 255 (0xFF). .Nm diff --git a/src/fix/main.cpp b/src/fix/main.cpp index 8f8766b2..8e636543 100644 --- a/src/fix/main.cpp +++ b/src/fix/main.cpp @@ -23,7 +23,7 @@ static_assert(UNSPECIFIED > 0xFF, "UNSPECIFIED should not be in byte range!"); static constexpr off_t BANK_SIZE = 0x4000; // Short options -static char const *optstring = "Ccf:hi:jk:L:l:m:n:Op:r:st:Vv"; +static char const *optstring = "Ccf:hi:jk:L:l:m:n:Oo:p:r:st:Vv"; // Equivalent long options // Please keep in the same order as short opts. @@ -45,6 +45,7 @@ static option const longopts[] = { {"mbc-type", required_argument, nullptr, 'm'}, {"rom-version", required_argument, nullptr, 'n'}, {"overwrite", no_argument, nullptr, 'O'}, + {"output", required_argument, nullptr, 'o'}, {"pad-value", required_argument, nullptr, 'p'}, {"ram-size", required_argument, nullptr, 'r'}, {"sgb-compatible", no_argument, nullptr, 's'}, @@ -66,6 +67,7 @@ static void printUsage() { " to the man page for a list of values\n" " -p, --pad-value pad to the next valid size using this value\n" " -r, --ram-size set the cart RAM size byte to this value\n" + " -o, --output set the output file\n" " -V, --version print RGBFIX version and exit\n" " -v, --validate fix the header logo and both checksums (-f lhg)\n" "\n" @@ -885,9 +887,9 @@ static void overwriteBytes( memcpy(&rom0[startAddr], fixed, size); } -static void processFile(int input, int output, char const *name, off_t fileSize) { - // Both of these should be true for seekable files, and neither otherwise - if (input == output) { +static void + processFile(int input, int output, char const *name, off_t fileSize, bool expectFileSize) { + if (expectFileSize) { assume(fileSize != 0); } else { assume(fileSize == 0); @@ -1224,21 +1226,48 @@ static void processFile(int input, int output, char const *name, off_t fileSize) } } -static bool processFilename(char const *name) { +static bool processFilename(char const *name, char const *outputName) { nbErrors = 0; - if (!strcmp(name, "-")) { + bool inputStdin = !strcmp(name, "-"); + if (inputStdin && !outputName) { + outputName = "-"; + } + + int output = -1; + bool openedOutput = false; + if (outputName) { + if (!strcmp(outputName, "-")) { + output = STDOUT_FILENO; + (void)setmode(STDOUT_FILENO, O_BINARY); + } else { + output = open(outputName, O_WRONLY | O_BINARY | O_CREAT, 0600); + if (output == -1) { + report( + "FATAL: Failed to open \"%s\" for writing: %s\n", outputName, strerror(errno) + ); + return true; + } + openedOutput = true; + } + } + Defer closeOutput{[&] { + if (openedOutput) { + close(output); + } + }}; + + if (inputStdin) { name = ""; (void)setmode(STDIN_FILENO, O_BINARY); - (void)setmode(STDOUT_FILENO, O_BINARY); - processFile(STDIN_FILENO, STDOUT_FILENO, name, 0); + processFile(STDIN_FILENO, output, name, 0, false); } else { // POSIX specifies that the results of O_RDWR on a FIFO are undefined. // However, this is necessary to avoid a TOCTTOU, if the file was changed between // `stat()` and `open(O_RDWR)`, which could trigger the UB anyway. // Thus, we're going to hope that either the `open` fails, or it succeeds but IO // operations may fail, all of which we handle. - if (int input = open(name, O_RDWR | O_BINARY); input == -1) { + if (int input = open(name, (outputName ? O_RDONLY : O_RDWR) | O_BINARY); input == -1) { report("FATAL: Failed to open \"%s\" for reading+writing: %s\n", name, strerror(errno)); } else { Defer closeInput{[&] { close(input); }}; @@ -1263,7 +1292,10 @@ static bool processFilename(char const *name) { static_cast(stat.st_size) ); } else { - processFile(input, input, name, stat.st_size); + if (!outputName) { + output = input; + } + processFile(input, output, name, stat.st_size, true); } } } @@ -1307,6 +1339,7 @@ static void parseByte(uint16_t &output, char name) { int main(int argc, char *argv[]) { nbErrors = 0; + char const *outputFilename = nullptr; for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, nullptr)) != -1;) { switch (ch) { size_t len; @@ -1421,6 +1454,10 @@ int main(int argc, char *argv[]) { overwriteRom = true; break; + case 'o': + outputFilename = musl_optarg; + break; + case 'p': parseByte(padValue, 'p'); break; @@ -1575,8 +1612,14 @@ int main(int argc, char *argv[]) { exit(1); } + if (outputFilename && argc != musl_optind + 1) { + fputs("FATAL: If `-o` is set then only a single input file may be specified\n", stderr); + printUsage(); + exit(1); + } + do { - failed |= processFilename(*argv); + failed |= processFilename(*argv, outputFilename); } while (*++argv); return failed; diff --git a/test/fix/test.sh b/test/fix/test.sh index 7fd5cbeb..bf39e95c 100755 --- a/test/fix/test.sh +++ b/test/fix/test.sh @@ -45,7 +45,7 @@ runTest () { sed "s# ./# ${src//#/\\#}/#g" # Prepend src directory to path arguments ) - for variant in '' ' piped'; do + for variant in '' ' piped' ' output'; do (( tests++ )) our_rc=0 if [[ $progress -ne 0 ]]; then @@ -63,13 +63,19 @@ runTest () { our_rc=1 fi subst=out.gb - else + 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' subst='' + elif [[ "$variant" = ' output' ]]; then + cp "$desired_input" input.gb + if [[ -n "$(eval "$RGBFIX" $flags -o out.gb input.gb '2>out.err')" ]]; then + our_rc=1 + fi + subst=input.gb fi if [[ -r "$2/$1.err" ]]; then