From 0d72ba886ba9125c861d15941b10494e24e6daca Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Thu, 2 Nov 2023 12:40:40 -0400 Subject: [PATCH] Consistent behavior with missing or incorrect parameters (#1179) --- man/rgbasm.1 | 3 +++ man/rgbfix.1 | 6 ++++++ man/rgblink.1 | 8 +++++++- src/asm/main.c | 32 +++++++++++++++++--------------- src/fix/main.c | 18 ++++++++++-------- src/gfx/main.cpp | 14 +++++++------- src/link/main.c | 21 ++++++++++----------- test/fix/test.sh | 2 +- 8 files changed, 61 insertions(+), 43 deletions(-) diff --git a/man/rgbasm.1 b/man/rgbasm.1 index 57ee0b67..a5aecb61 100644 --- a/man/rgbasm.1 +++ b/man/rgbasm.1 @@ -34,6 +34,9 @@ The .Nm program creates an RGB object file from an assembly source file. +The object file format is documented in +.Xr rgbds 5 . +.Pp The input .Ar asmfile can be a path to a file, or diff --git a/man/rgbfix.1 b/man/rgbfix.1 index 0e29220d..416c3b32 100644 --- a/man/rgbfix.1 +++ b/man/rgbfix.1 @@ -41,6 +41,12 @@ Developers are advised to fill those fields with 0x00 bytes in their source code and to have already populated whichever fields they don't specify using .Nm . .Pp +The input +.Ar asmfile +can be a path to a file, or +.Cm \- +to read from standard input. +.Pp Note that options can be abbreviated as long as the abbreviation is unambiguous: .Fl Fl color-o is diff --git a/man/rgblink.1 b/man/rgblink.1 index 7e849602..0537a9eb 100644 --- a/man/rgblink.1 +++ b/man/rgblink.1 @@ -29,7 +29,7 @@ The program links RGB object files, typically created by .Xr rgbasm 1 , into a single Game Boy ROM file. -The format is documented in +The object file format is documented in .Xr rgbds 5 . .Pp ROM0 sections are placed in the first 16 KiB of the output ROM, and ROMX sections are placed in any 16 KiB @@ -52,6 +52,12 @@ option, which implies .Fl w but also prohibits the use of banked VRAM. .Pp +The input +.Ar asmfile +can be a path to a file, or +.Cm \- +to read from standard input. +.Pp Note that options can be abbreviated as long as the abbreviation is unambiguous: .Fl Fl verb is diff --git a/src/asm/main.c b/src/asm/main.c index a1d7b989..038dfab2 100644 --- a/src/asm/main.c +++ b/src/asm/main.c @@ -126,7 +126,7 @@ static struct option const longopts[] = { { NULL, no_argument, NULL, 0 } }; -static void print_usage(void) +static void printUsage(void) { fputs( "Usage: rgbasm [-EHhLlVvw] [-b chars] [-D name[=value]] [-g chars] [-I path]\n" @@ -143,7 +143,6 @@ static void print_usage(void) "\n" "For help, use `man rgbasm' or go to https://rgbds.gbdev.io/docs/\n", stderr); - exit(1); } int main(int argc, char *argv[]) @@ -176,10 +175,10 @@ int main(int argc, char *argv[]) char *dependFileName = NULL; size_t targetFileNameLen = 0; - int ch; - char *ep; - while ((ch = musl_getopt_long_only(argc, argv, optstring, longopts, NULL)) != -1) { + for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, NULL)) != -1;) { switch (ch) { + char *endptr; + case 'b': if (strlen(musl_optarg) == 2) opt_B(musl_optarg); @@ -263,9 +262,9 @@ int main(int argc, char *argv[]) unsigned long padByte; case 'p': - padByte = strtoul(musl_optarg, &ep, 0); + padByte = strtoul(musl_optarg, &endptr, 0); - if (musl_optarg[0] == '\0' || *ep != '\0') + if (musl_optarg[0] == '\0' || *endptr != '\0') errx("Invalid argument for option 'p'"); if (padByte > 0xFF) @@ -280,9 +279,9 @@ int main(int argc, char *argv[]) precisionArg = musl_optarg; if (precisionArg[0] == '.') precisionArg++; - precision = strtoul(precisionArg, &ep, 0); + precision = strtoul(precisionArg, &endptr, 0); - if (musl_optarg[0] == '\0' || *ep != '\0') + if (musl_optarg[0] == '\0' || *endptr != '\0') errx("Invalid argument for option 'Q'"); if (precision < 1 || precision > 31) @@ -292,9 +291,9 @@ int main(int argc, char *argv[]) break; case 'r': - maxDepth = strtoul(musl_optarg, &ep, 0); + maxDepth = strtoul(musl_optarg, &endptr, 0); - if (musl_optarg[0] == '\0' || *ep != '\0') + if (musl_optarg[0] == '\0' || *endptr != '\0') errx("Invalid argument for option 'r'"); break; @@ -348,8 +347,9 @@ int main(int argc, char *argv[]) // Unrecognized options default: - print_usage(); - // NOTREACHED + fprintf(stderr, "FATAL: unknown option '%c'\n", ch); + printUsage(); + exit(1); } } @@ -360,10 +360,12 @@ int main(int argc, char *argv[]) if (argc == musl_optind) { fputs("FATAL: Please specify an input file (pass `-` to read from standard input)\n", stderr); - print_usage(); + printUsage(); + exit(1); } else if (argc != musl_optind + 1) { fputs("FATAL: More than one input file specified\n", stderr); - print_usage(); + printUsage(); + exit(1); } char const *mainFileName = argv[musl_optind]; diff --git a/src/fix/main.c b/src/fix/main.c index d0a0fc25..d603452d 100644 --- a/src/fix/main.c +++ b/src/fix/main.c @@ -67,7 +67,7 @@ static void printUsage(void) fputs( "Usage: rgbfix [-jOsVv] [-C | -c] [-f ] [-i ] [-k ]\n" " [-l ] [-m ] [-n ]\n" -" [-p ] [-r ] [-t ] [ ...]\n" +" [-p ] [-r ] [-t ] ...\n" "Useful options:\n" " -m, --mbc-type set the MBC type byte to this value; refer\n" " to the man page for a list of values\n" @@ -1224,9 +1224,8 @@ finish: int main(int argc, char *argv[]) { nbErrors = 0; - int ch; - while ((ch = musl_getopt_long_only(argc, argv, optstring, longopts, NULL)) != -1) { + for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, NULL)) != -1;) { switch (ch) { size_t len; #define parseByte(output, name) \ @@ -1447,12 +1446,15 @@ do { \ bool failed = nbErrors; if (!*argv) { - failed |= processFilename("-"); - } else { - do { - failed |= processFilename(*argv); - } while (*++argv); + fputs("FATAL: Please specify an input file (pass `-` to read from standard input)\n", + stderr); + printUsage(); + exit(1); } + do { + failed |= processFilename(*argv); + } while (*++argv); + return failed; } diff --git a/src/gfx/main.cpp b/src/gfx/main.cpp index 37730527..57a8177a 100644 --- a/src/gfx/main.cpp +++ b/src/gfx/main.cpp @@ -153,7 +153,6 @@ static void printUsage(void) { "\n" "For help, use `man rgbgfx' or go to https://rgbds.gbdev.io/docs/\n", stderr); - exit(1); } /* @@ -333,11 +332,9 @@ static std::vector readAtFile(std::string const &path, std::vector */ static char *parseArgv(int argc, char **argv, bool &autoAttrmap, bool &autoTilemap, bool &autoPalettes, bool &autoPalmap) { - int opt; - - while ((opt = musl_getopt_long_only(argc, argv, optstring, longopts, nullptr)) != -1) { + for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, nullptr)) != -1;) { char *arg = musl_optarg; // Make a copy for scanning - switch (opt) { + switch (ch) { case 'A': autoAttrmap = true; break; @@ -569,9 +566,10 @@ static char *parseArgv(int argc, char **argv, bool &autoAttrmap, bool &autoTilem case 'D': case 'F': case 'f': - warning("Ignoring retired option `-%c`", opt); + warning("Ignoring retired option `-%c`", ch); break; default: + fprintf(stderr, "FATAL: unknown option '%c'\n", ch); printUsage(); exit(1); } @@ -769,7 +767,9 @@ int main(int argc, char *argv[]) { } if (options.input.empty()) { - fatal("No input image specified"); + fputs("FATAL: No input image specified\n", stderr); + printUsage(); + exit(1); } // Do not do anything if option parsing went wrong diff --git a/src/link/main.c b/src/link/main.c index 7ad9dad8..74b0393c 100644 --- a/src/link/main.c +++ b/src/link/main.c @@ -196,7 +196,6 @@ static struct option const longopts[] = { { NULL, no_argument, NULL, 0 } }; -// Prints the program's usage to stdout. static void printUsage(void) { fputs( @@ -356,14 +355,9 @@ _Noreturn void reportErrors(void) { int main(int argc, char *argv[]) { - int optionChar; - char *endptr; // For error checking with `strtoul` - unsigned long value; // For storing `strtoul`'s return value - // Parse options - while ((optionChar = musl_getopt_long_only(argc, argv, optstring, - longopts, NULL)) != -1) { - switch (optionChar) { + for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, NULL)) != -1;) { + switch (ch) { case 'd': isDmgMode = true; isWRA0Mode = true; @@ -396,8 +390,10 @@ int main(int argc, char *argv[]) warnx("Overriding output file %s", musl_optarg); outputFileName = musl_optarg; break; - case 'p': - value = strtoul(musl_optarg, &endptr, 0); + case 'p': { + char *endptr; + unsigned long value = strtoul(musl_optarg, &endptr, 0); + if (musl_optarg[0] == '\0' || *endptr != '\0') { argErr('p', ""); value = 0xFF; @@ -408,6 +404,7 @@ int main(int argc, char *argv[]) } padValue = value; break; + } case 'S': parseScrambleSpec(musl_optarg); break; @@ -434,6 +431,7 @@ int main(int argc, char *argv[]) is32kMode = true; break; default: + fprintf(stderr, "FATAL: unknown option '%c'\n", ch); printUsage(); exit(1); } @@ -443,7 +441,8 @@ int main(int argc, char *argv[]) // If no input files were specified, the user must have screwed up if (curArgIndex == argc) { - fputs("FATAL: no input files\n", stderr); + fputs("FATAL: Please specify an input file (pass `-` to read from standard input)\n", + stderr); printUsage(); exit(1); } diff --git a/test/fix/test.sh b/test/fix/test.sh index bd30524f..7c008fcb 100755 --- a/test/fix/test.sh +++ b/test/fix/test.sh @@ -52,7 +52,7 @@ runTest () { # 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. - cat "$2/$1.bin" | eval $RGBFIX "$flags" '>out.gb' '2>out.err' + cat "$2/$1.bin" | eval $RGBFIX "$flags" - '>out.gb' '2>out.err' subst='' fi