From 28deb818af0420ab52a097d0640b617cc9435e43 Mon Sep 17 00:00:00 2001 From: bradenbest Date: Sun, 26 May 2024 00:30:44 -0600 Subject: [PATCH] 1.7 - Better arg parsing - Heavily refactored arg parsing to use a simple search-and-dispatch pattern - The argument order no longer matters. The last filename entered is the one used. - longopts can now take arguments in --key=value or --key value format - Added -c to --color - -h/--help is now a canon option - -- now disables opt parsing instead of merely exiting the arg parsing loop - updated man page to explain what -s/--sector does - made an effort to keep code style as similar to pixel's as possible (next line brace, 2-space indent) Feel free to review my changes. I'm not used to automake/m4, but I think I got everything. I even logged my changes in the changelog, which I've duplicated above (that's just how I format my commits). I made an effort to keep the style as similar to yours as possible. I observed camelCase, 2-space indent and next line braces. I did break the rules in a couple places, namely parseArg_matchOpt. The underscores are a "pseudo namespace". I think it's reasonable. I also added one type, ParseArgCallbackFn, in PascalCase because I assume that's the convention (PascalCase for types, camelCase for functions and variables). I tried to keep the changes small in scope, which is why the LUT in parseArg is an anonymous struct and why there aren't any enums, like for the return value of parseArg_matchOpt. The new arg parser loop doesn't behave completely identically to the old one, but the changes I made are sane and reasonable. For example, the order no longer matters, you can do `filename -s` or `-s filename`. There was also some stipulation about throwing the usage if argc > 1 after the arg parser loop is done. I did away with that. I was careful to preserve the behavior when fileName is NULL: the program will make one attempt to obtain it interactively (findFile), and then either exit with an error message or open the file. That's about all that comes to mind. Nice app! -Braden Best (bradenbest) --- Changes | 11 +++ configure.ac | 2 +- hexedit.c | 224 +++++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 193 insertions(+), 44 deletions(-) diff --git a/Changes b/Changes index 43d9062..ca6fcea 100644 --- a/Changes +++ b/Changes @@ -1,3 +1,14 @@ +may 2024 + - 1.7 + - Heavily refactored arg parsing to use a simple search-and-dispatch pattern + - The argument order no longer matters. The last filename entered is the one used. + - longopts can now take arguments in --key=value or --key value format + - Added -c to --color + - -h/--help is now a canon option + - -- now disables opt parsing instead of merely exiting the arg parsing loop + - updated man page to explain what -s/--sector does + - made an effort to keep code style as similar to pixel's as possible (next line brace, 2-space indent) + april 2022 - 1.6 - configure script must error-out when (n)curses isn't found diff --git a/configure.ac b/configure.ac index c5b05f3..0b11886 100644 --- a/configure.ac +++ b/configure.ac @@ -9,7 +9,7 @@ AC_INIT AC_CONFIG_SRCDIR([hexedit.c]) AC_CONFIG_HEADERS(config.h) -define(TheVERSION, 1.6) +define(TheVERSION, 1.7) PRODUCT=hexedit VERSION=TheVERSION diff --git a/hexedit.c b/hexedit.c index c00515d..d36f1de 100644 --- a/hexedit.c +++ b/hexedit.c @@ -41,13 +41,182 @@ const modeParams modes[LAST] = { modeType mode = maximized; int colored = FALSE; int isReadOnly = FALSE; +int disableOpts = FALSE; const char * const usage = "usage: %s [-s | --sector] [-m | --maximize] [-l | --linelength ] [-r | --readonly]" #ifdef HAVE_COLORS - " [--color]" + " [-c | --color]" #endif " [-h | --help] filename\n"; +typedef void (*ParseArgCallbackFn)(const char *nextArg); + +static void pacallback_sector (const char *unused); +static void pacallback_readOnly (const char *unused); +static void pacallback_maximize (const char *unused); +#ifdef HAVE_COLORS +static void pacallback_color (const char *unused); +#endif +static void pacallback_lineLength (const char *nextArg); +static void pacallback_disableOpts (const char *unused); +static void pacallback_usage (const char *unused); + +static inline int parseArg_matchOpt (const char *arg, const char *shortName, const char *longName); +static inline int parseArg (const char **argv); + +/* name: pacallback_* + * nextArg: argument for option, if applicable + * desc: these are callbacks per ParseArgCallbackFn, used by parseArg + */ +static void pacallback_sector(const char *unused) +{ + (void) unused; + mode = bySector; +} + +static void pacallback_readOnly(const char *unused) +{ + (void) unused; + isReadOnly = TRUE; +} + +static void pacallback_maximize(const char *unused) +{ + (void) unused; + mode = maximized; + lineLength = 0; +} + +#ifdef HAVE_COLORS +static void pacallback_color(const char *unused) +{ + (void) unused; + colored = TRUE; +} +#endif + +static void pacallback_lineLength(const char *nextArg) +{ + lineLength = atoi(nextArg); + + if (lineLength < 0 || lineLength > 4096) + DIE("%s: illegal line length\n") +} + +static void pacallback_disableOpts(const char *unused) +{ + (void) unused; + disableOpts = TRUE; +} + +static void pacallback_usage(const char *unused) +{ + (void) unused; + DIE(usage); +} + +/* name: parseArg_matchOpt + * arg: current argument + * shortName: short name to check against + * longName: long name to check against + * return: number indicating status + * 0: no match + * 1: short match + * 2: long match + */ +static inline int parseArg_matchOpt(const char *arg, const char *shortName, const char *longName) +{ + if (shortName != NULL && strbeginswith(arg, shortName)) + return 1; + + if (longName != NULL && strbeginswith(arg, longName)) + return 2; + + return 0; +} + +/* name: parseArg + * argvp: pointer to current argv offset + * return: number of arguments to advance + * desc: parses arguments according to embedded argument list. + * Execution is handled by callback functions. + * notes: + * + * 1. This can be canonized as a type in a future update, but is left + * anonymous for now + * + * 2. Adds support for the --longopt=value convention + * + * 3. if a filename was already selected, free the existing one and + * replace it. Note that file scope variables are guaranteed to be + * zero initialized per the standard. + * + * 4. Fixes issue #70 -- if no options match the argument, it is assumed + * to be the filename. The effect is that the most recent "naked" + * argument is used as the filename. + */ +static inline int parseArg(const char **argvp) +{ + static const struct { // 1 + const char * shortName; + const char * longName; + int hasArg; + ParseArgCallbackFn callback; + } opts[] = { + {"-s", "--sector", FALSE, pacallback_sector}, + {"-r", "--readonly", FALSE, pacallback_readOnly}, + {"-m", "--maximize", FALSE, pacallback_maximize}, +#ifdef HAVE_COLORS + {"-c", "--color", FALSE, pacallback_color}, +#endif + {"-l", "--linelength", TRUE, pacallback_lineLength}, + {"-h", "--help", FALSE, pacallback_usage}, + {"--", NULL, FALSE, pacallback_disableOpts}, + {"-", NULL, FALSE, pacallback_usage}, + }; + static const size_t opts_len = sizeof opts / sizeof *opts; + + const char *curArg = argvp[0]; + const char *nextArg = NULL; + int advanceBy = 1; + + if (disableOpts) + goto exit_disableOpts; + + for (size_t i = 0; i < opts_len; ++i) + { + int matchOpt = parseArg_matchOpt(curArg, opts[i].shortName, opts[i].longName); + const char *strchrMatch; + + if (matchOpt == 0) + continue; + + if (opts[i].hasArg) + { + if (matchOpt == 1 && strlen(curArg) > 2) + nextArg = curArg + 2; + + else if (matchOpt == 2 && (strchrMatch = strchr(curArg, '=')) != NULL) // 2 + nextArg = strchrMatch + 1; + + else + { + nextArg = argvp[1]; + advanceBy = 2; + } + } + + opts[i].callback(nextArg); + return advanceBy; + } + +exit_disableOpts: + if (fileName != NULL) // 3 + free(fileName); + + fileName = strdup(curArg); // 4 + return 1; +} /*******************************************************************************/ /* main */ @@ -57,52 +226,21 @@ int main(int argc, char **argv) progName = basename(argv[0]); argv++; argc--; - for (; argc > 0; argv++, argc--) - { - if (streq(*argv, "-s") || streq(*argv, "--sector")) - mode = bySector; - else if (streq(*argv, "-r") || streq(*argv, "--readonly")) - isReadOnly = TRUE; - else if (streq(*argv, "-m") || streq(*argv, "--maximize")) { - mode = maximized; - lineLength = 0; - } -#ifdef HAVE_COLORS - else if (streq(*argv, "--color")) - colored = TRUE; -#endif - else if (strbeginswith(*argv, "-l") || strbeginswith(*argv, "--linelength")) { - if (strbeginswith(*argv, "-l") && strlen(*argv) > 2) - lineLength = atoi(*argv + 2); - else { - argv++; argc--; - lineLength = atoi(*argv); - } - if (lineLength < 0 || lineLength > 4096) - DIE("%s: illegal line length\n") - } else if (streq(*argv, "--")) { - argv++; argc--; - break; - } else if (*argv[0] == '-') - - DIE(usage) - else break; - } - if (argc > 1) DIE(usage); + for (; argc > 0;) + { + int advanceBy = parseArg((const char **)argv); - init(); - if (argc == 1) { - fileName = strdup(*argv); - openFile(); + argv += advanceBy; + argc -= advanceBy; } + + init(); initCurses(); - if (fileName == NULL) { - if (!findFile()) { - exitCurses(); - DIE("%s: No such file\n"); - } - openFile(); + if (fileName == NULL && !findFile()) { + exitCurses(); + DIE("%s: No such file\n"); } + openFile(); readFile(); do display(); while (key_to_function(getch()));