From e0a2175c2e0e18367b0378163e822a276b05d3b0 Mon Sep 17 00:00:00 2001 From: Andy Grover Date: Fri, 4 Dec 2020 14:32:59 -0800 Subject: [PATCH] Use argp info instead of duplicating for cmd_register() Make it static and then use it both for argp_parse as well as cmd_register_argp. Split commands into five groups, to help understanding of their usefulness. Mention that each command has its own help text, and that we are being fancy to keep the user from having to give fs path. Signed-off-by: Andy Grover --- utils/src/cmd.c | 64 ++++++++++++++++++++++++------------ utils/src/cmd.h | 8 ++++- utils/src/counters.c | 17 +++++----- utils/src/df.c | 16 ++++----- utils/src/ino_path.c | 16 ++++----- utils/src/listxattr_hidden.c | 17 +++++----- utils/src/main.c | 1 + utils/src/mkfs.c | 15 +++++---- utils/src/print.c | 16 ++++----- utils/src/search_xattrs.c | 18 +++++----- utils/src/setattr.c | 17 +++++----- utils/src/stage_release.c | 30 ++++++++--------- utils/src/stat.c | 37 +++++++++++---------- utils/src/util.c | 4 ++- utils/src/waiting.c | 31 +++++++---------- utils/src/walk_inodes.c | 15 +++++---- 16 files changed, 172 insertions(+), 150 deletions(-) diff --git a/utils/src/cmd.c b/utils/src/cmd.c index 607f12ec..a5e590aa 100644 --- a/utils/src/cmd.c +++ b/utils/src/cmd.c @@ -4,35 +4,38 @@ #include #include #include +#include #include "cmd.h" #include "util.h" -static struct command { +static struct argp_command { char *name; - char *opts; - char *summary; + struct argp *argp; + int group; + char __pad[4]; int (*func)(int argc, char **argv); -} cmds[100], *next_cmd = cmds; +} argp_cmds[100], *next_argp_cmd = argp_cmds; -#define cmd_for_each(com) for (com = cmds; com->func; com++) +#define cmd_for_each(com) for (com = argp_cmds; com->func; com++) -void cmd_register(char *name, char *opts, char *summary, +void cmd_register_argp(char *name, struct argp *argp, int group, int (*func)(int argc, char **argv)) { - struct command *com = next_cmd++; + struct argp_command *com = next_argp_cmd++; - assert((com - cmds) < array_size(cmds)); + assert((com - argp_cmds) < array_size(argp_cmds)); com->name = name; - com->opts = opts; - com->summary = summary; + com->argp = argp; + com->group = group; com->func = func; } -static struct command *find_command(char *name) + +static struct argp_command *find_command(char *name) { - struct command *com; + struct argp_command *com; cmd_for_each(com) { if (!strcmp(name, com->name)) @@ -42,28 +45,47 @@ static struct command *find_command(char *name) return NULL; } -static void usage(void) +static void print_cmds_for_group(int group) { - struct command *com; + struct argp_command *com; int largest = 0; - fprintf(stderr, "usage: scoutfs []\n" - "Commands:\n"); - + /* Base alignment on all groups */ cmd_for_each(com) largest = max(strlen(com->name), largest); cmd_for_each(com) { - fprintf(stderr, " %*s %s\n %*s %s\n", - largest, com->name, com->opts, - largest, "", com->summary); + if (com->group == group) { + fprintf(stderr, " %*s %s\n %*s %s\n", + largest, com->name, com->argp->args_doc, + largest, "", com->argp->doc); + } } + +} + +static void usage(void) +{ + fprintf(stderr, "usage: scoutfs []\n\n"); + fprintf(stderr, "Selected fs defaults to current working directory.\n"); + fprintf(stderr, "See --help for more details.\n"); + + fprintf(stderr, "\nCore admin:\n"); + print_cmds_for_group(GROUP_CORE); + fprintf(stderr, "\nAdditional Information:\n"); + print_cmds_for_group(GROUP_INFO); + fprintf(stderr, "\nSearch Acceleration:\n"); + print_cmds_for_group(GROUP_SEARCH); + fprintf(stderr, "\nArchival Agent Support:\n"); + print_cmds_for_group(GROUP_AGENT); + fprintf(stderr, "\nDebugging commands:\n"); + print_cmds_for_group(GROUP_DEBUG); } /* this returns a positive unix return code on error for some reason */ char cmd_execute(int argc, char **argv) { - struct command *com = NULL; + struct argp_command *com = NULL; int ret; if (argc > 1) { diff --git a/utils/src/cmd.h b/utils/src/cmd.h index 084590e9..b225e22e 100644 --- a/utils/src/cmd.h +++ b/utils/src/cmd.h @@ -1,7 +1,13 @@ #ifndef _CMD_H_ #define _CMD_H_ -void cmd_register(char *name, char *opts, char *summary, +#define GROUP_CORE 0 +#define GROUP_INFO 1 +#define GROUP_SEARCH 2 +#define GROUP_AGENT 3 +#define GROUP_DEBUG 4 + +void cmd_register_argp(char *name, struct argp *argp, int group, int (*func)(int argc, char **argv)); char cmd_execute(int argc, char **argv); diff --git a/utils/src/counters.c b/utils/src/counters.c index b57ecb54..c71fa8bd 100644 --- a/utils/src/counters.c +++ b/utils/src/counters.c @@ -303,14 +303,15 @@ static struct argp_option options[] = { { NULL } }; +static struct argp argp = { + options, + parse_opt, + "SYSFS-DIR", + "Show counters for a mounted volume" +}; + static int counters_cmd(int argc, char *argv[]) { - struct argp argp = { - options, - parse_opt, - "SYSFS-DIR", - "Show counters for a mounted volume" - }; struct counters_args counters_args = {NULL}; int ret; @@ -324,7 +325,5 @@ static int counters_cmd(int argc, char *argv[]) static void __attribute__((constructor)) counters_ctor(void) { - cmd_register("counters", "[-t] ", - "show [tabular] counters for a given mounted volume", - counters_cmd); + cmd_register_argp("counters", &argp, GROUP_INFO, counters_cmd); } diff --git a/utils/src/df.c b/utils/src/df.c index 4c8be0ef..a6ca0d12 100644 --- a/utils/src/df.c +++ b/utils/src/df.c @@ -175,14 +175,15 @@ static struct argp_option options[] = { { NULL } }; +static struct argp argp = { + options, + parse_opt, + "", + "Show metadata and data block usage" +}; + static int df_cmd(int argc, char **argv) { - struct argp argp = { - options, - parse_opt, - NULL, - "Show metadata and data block usage" - }; struct df_args df_args = {NULL}; int ret; @@ -196,6 +197,5 @@ static int df_cmd(int argc, char **argv) static void __attribute__((constructor)) df_ctor(void) { - cmd_register("df", "", - "Show metadata and data block usage", df_cmd); + cmd_register_argp("df", &argp, GROUP_CORE, df_cmd); } diff --git a/utils/src/ino_path.c b/utils/src/ino_path.c index aaa00f2d..99d05de2 100644 --- a/utils/src/ino_path.c +++ b/utils/src/ino_path.c @@ -113,14 +113,15 @@ static struct argp_option options[] = { { NULL } }; +static struct argp argp = { + options, + parse_opt, + "INODE-NUM", + "Print paths that refer to inode number" +}; + static int ino_path_cmd(int argc, char **argv) { - struct argp argp = { - options, - parse_opt, - "INODE-NUM", - "Print paths that refer to inode number" - }; struct ino_args ino_args = {NULL}; int ret; @@ -134,6 +135,5 @@ static int ino_path_cmd(int argc, char **argv) static void __attribute__((constructor)) ino_path_ctor(void) { - cmd_register("ino-path", " ", - "print paths that refer to inode #", ino_path_cmd); + cmd_register_argp("ino-path", &argp, GROUP_SEARCH, ino_path_cmd); } diff --git a/utils/src/listxattr_hidden.c b/utils/src/listxattr_hidden.c index 10130b3c..da389ab2 100644 --- a/utils/src/listxattr_hidden.c +++ b/utils/src/listxattr_hidden.c @@ -137,14 +137,15 @@ static int parse_opt(int key, char *arg, struct argp_state *state) return 0; } +static struct argp argp = { + NULL, + parse_opt, + "FILE", + "Print the names of hidden xattrs on a file" +}; + static int list_hidden_xattrs_cmd(int argc, char **argv) { - struct argp argp = { - NULL, - parse_opt, - "FILE", - "Print the names of hidden xattrs on a file" - }; struct list_hidden_xattr_args list_hidden_xattr_args = {NULL}; int ret; @@ -158,7 +159,5 @@ static int list_hidden_xattrs_cmd(int argc, char **argv) static void __attribute__((constructor)) listxattr_hidden_ctor(void) { - cmd_register("list-hidden-xattrs", "", - "print the names of hidden xattrs on a file", - list_hidden_xattrs_cmd); + cmd_register_argp("list-hidden-xattrs", &argp, GROUP_INFO, list_hidden_xattrs_cmd); } diff --git a/utils/src/main.c b/utils/src/main.c index 369b4ece..277b8ec2 100644 --- a/utils/src/main.c +++ b/utils/src/main.c @@ -4,6 +4,7 @@ #include #include #include +#include #include "cmd.h" #include "util.h" diff --git a/utils/src/mkfs.c b/utils/src/mkfs.c index d9c04dd4..2c1936fb 100644 --- a/utils/src/mkfs.c +++ b/utils/src/mkfs.c @@ -455,14 +455,15 @@ static struct argp_option options[] = { { NULL } }; +static struct argp argp = { + options, + parse_opt, + "META-DEVICE DATA-DEVICE", + "Initialize a new ScoutFS filesystem" +}; + static int mkfs_cmd(int argc, char *argv[]) { - struct argp argp = { - options, - parse_opt, - "META-DEVICE DATA-DEVICE", - "Initialize a new ScoutFS filesystem" - }; struct mkfs_args mkfs_args = {0}; int ret; @@ -475,7 +476,7 @@ static int mkfs_cmd(int argc, char *argv[]) static void __attribute__((constructor)) mkfs_ctor(void) { - cmd_register("mkfs", " ", "write a new file system", mkfs_cmd); + cmd_register_argp("mkfs", &argp, GROUP_CORE, mkfs_cmd); /* for lack of some other place to put these.. */ build_assert(sizeof(uuid_t) == SCOUTFS_UUID_BYTES); diff --git a/utils/src/print.c b/utils/src/print.c index faacb467..aff6110a 100644 --- a/utils/src/print.c +++ b/utils/src/print.c @@ -1061,14 +1061,15 @@ static int parse_opt(int key, char *arg, struct argp_state *state) return 0; } +static struct argp argp = { + NULL, + parse_opt, + "META-DEV", + "Print metadata structures" +}; + static int print_cmd(int argc, char **argv) { - struct argp argp = { - NULL, - parse_opt, - "META-DEV", - "Print metadata structures" - }; struct print_args print_args = {NULL}; int ret; @@ -1082,6 +1083,5 @@ static int print_cmd(int argc, char **argv) static void __attribute__((constructor)) print_ctor(void) { - cmd_register("print", "", "print metadata structures", - print_cmd); + cmd_register_argp("print", &argp, GROUP_DEBUG, print_cmd); } diff --git a/utils/src/search_xattrs.c b/utils/src/search_xattrs.c index a079a13f..bb242f06 100644 --- a/utils/src/search_xattrs.c +++ b/utils/src/search_xattrs.c @@ -112,14 +112,16 @@ static struct argp_option options[] = { { NULL } }; +static struct argp argp = { + options, + parse_opt, + "XATTR-NAME", + "Print inode numbers of inodes which may have given xattr" +}; + static int search_xattrs_cmd(int argc, char **argv) { - struct argp argp = { - options, - parse_opt, - "XATTR-NAME", - "Print inode numbers of inodes which may have given xattr" - }; + struct xattr_args xattr_args = {NULL}; int ret; @@ -132,7 +134,5 @@ static int search_xattrs_cmd(int argc, char **argv) static void __attribute__((constructor)) search_xattrs_ctor(void) { - cmd_register("search-xattrs", "-n name -f ", - "print inode numbers of inodes which may have given xattr", - search_xattrs_cmd); + cmd_register_argp("search-xattrs", &argp, GROUP_INFO, search_xattrs_cmd); } diff --git a/utils/src/setattr.c b/utils/src/setattr.c index 23922531..e4178f6c 100644 --- a/utils/src/setattr.c +++ b/utils/src/setattr.c @@ -120,14 +120,15 @@ static struct argp_option options[] = { { NULL } }; +static struct argp argp = { + options, + parse_opt, + "FILE", + "Set attributes on newly-created zero-length file" +}; + static int setattr_cmd(int argc, char **argv) { - struct argp argp = { - options, - parse_opt, - "FILE", - "Set attributes on newly-created zero-length file" - }; struct setattr_args setattr_args = {NULL}; int ret; @@ -140,7 +141,5 @@ static int setattr_cmd(int argc, char **argv) static void __attribute__((constructor)) setattr_more_ctor(void) { - cmd_register("setattr", "", - "set attributes on newly-created zero-length file", - setattr_cmd); + cmd_register_argp("setattr", &argp, GROUP_AGENT, setattr_cmd); } diff --git a/utils/src/stage_release.c b/utils/src/stage_release.c index 8683afd1..38b62196 100644 --- a/utils/src/stage_release.c +++ b/utils/src/stage_release.c @@ -163,18 +163,19 @@ static struct argp_option options[] = { { NULL } }; -static int stage_cmd(int argc, char **argv) -{ - struct argp argp = { +static struct argp stage_argp = { options, parse_stage_opts, "ARCHIVE-FILE STAGE-FILE --data-version VERSION", "Write archive file contents to an offline file" }; + +static int stage_cmd(int argc, char **argv) +{ struct stage_args stage_args = {NULL}; int ret; - ret = argp_parse(&argp, argc, argv, 0, NULL, &stage_args); + ret = argp_parse(&stage_argp, argc, argv, 0, NULL, &stage_args); if (ret) return ret; @@ -183,8 +184,7 @@ static int stage_cmd(int argc, char **argv) static void __attribute__((constructor)) stage_ctor(void) { - cmd_register("stage", " -V ", - "write archive file contents to an offline file", stage_cmd); + cmd_register_argp("stage", &stage_argp, GROUP_AGENT, stage_cmd); } struct release_args { @@ -277,18 +277,19 @@ static int parse_release_opts(int key, char *arg, struct argp_state *state) return 0; } +static struct argp release_argp = { + options, + parse_release_opts, + "FILE --data-version VERSION", + "Mark file region offline and free extents" +}; + static int release_cmd(int argc, char **argv) { - struct argp argp = { - options, - parse_release_opts, - "FILE --data-version VERSION", - "Mark file region offline and free extents" - }; struct release_args release_args = {NULL}; int ret; - ret = argp_parse(&argp, argc, argv, 0, NULL, &release_args); + ret = argp_parse(&release_argp, argc, argv, 0, NULL, &release_args); if (ret) return ret; @@ -297,6 +298,5 @@ static int release_cmd(int argc, char **argv) static void __attribute__((constructor)) release_ctor(void) { - cmd_register("release", " ", - "mark file region offline and free extents", release_cmd); + cmd_register_argp("release", &release_argp, GROUP_AGENT, release_cmd); } diff --git a/utils/src/stat.c b/utils/src/stat.c index 3e20d1d0..996d4106 100644 --- a/utils/src/stat.c +++ b/utils/src/stat.c @@ -202,18 +202,19 @@ static struct argp_option stat_options[] = { { NULL } }; +static struct argp stat_argp = { + stat_options, + stat_parse_opt, + "FILE", + "Show ScoutFS extra inode information" +}; + static int stat_more_cmd(int argc, char **argv) { - struct argp argp = { - stat_options, - stat_parse_opt, - "FILE", - "Show ScoutFS extra inode information" - }; struct stat_args stat_args = {NULL}; int ret; - ret = argp_parse(&argp, argc, argv, 0, NULL, &stat_args); + ret = argp_parse(&stat_argp, argc, argv, 0, NULL, &stat_args); if (ret) return ret; stat_args.is_inode = true; @@ -244,18 +245,20 @@ static int statfs_parse_opt(int key, char *arg, struct argp_state *state) return 0; } + +static struct argp statfs_argp = { + statfs_options, + statfs_parse_opt, + "", + "Show ScoutFS file system information" +}; + static int statfs_more_cmd(int argc, char **argv) { - struct argp argp = { - statfs_options, - statfs_parse_opt, - NULL, - "Show ScoutFS file system information" - }; struct stat_args stat_args = {NULL}; int ret; - ret = argp_parse(&argp, argc, argv, 0, NULL, &stat_args); + ret = argp_parse(&statfs_argp, argc, argv, 0, NULL, &stat_args); if (ret) return ret; stat_args.is_inode = false; @@ -265,12 +268,10 @@ static int statfs_more_cmd(int argc, char **argv) static void __attribute__((constructor)) stat_more_ctor(void) { - cmd_register("stat", "", - "show scoutfs inode information", stat_more_cmd); + cmd_register_argp("stat", &stat_argp, GROUP_INFO, stat_more_cmd); } static void __attribute__((constructor)) statfs_more_ctor(void) { - cmd_register("statfs", "", - "show scoutfs file system information", statfs_more_cmd); + cmd_register_argp("statfs", &statfs_argp, GROUP_INFO, statfs_more_cmd); } diff --git a/utils/src/util.c b/utils/src/util.c index d30e44ba..9c3aa6ef 100644 --- a/utils/src/util.c +++ b/utils/src/util.c @@ -11,6 +11,8 @@ #include "util.h" +#define ENV_PATH "SCOUTFS_MOUNT_PATH" + static int open_path(char *path, int flags) { wordexp_t exp_result; @@ -51,7 +53,7 @@ int get_path(char *path, int flags) if (path) return open_path(path, flags); - env_path = getenv("SCOUTFS_PATH"); + env_path = getenv(ENV_PATH); if (env_path) return open_path(path, flags); diff --git a/utils/src/waiting.c b/utils/src/waiting.c index 35abdd8d..92b31f6b 100644 --- a/utils/src/waiting.c +++ b/utils/src/waiting.c @@ -131,18 +131,19 @@ static struct argp_option waiting_options[] = { { NULL } }; +static struct argp waiting_argp = { + waiting_options, + waiting_parse_opt, + "--inode INODE-NUM --block BLOCK-NUM", + "Print operations waiting for data blocks" +}; + static int waiting_cmd(int argc, char **argv) { - struct argp argp = { - waiting_options, - waiting_parse_opt, - "--inode INODE-NUM --block BLOCK-NUM", - "Print operations waiting for data blocks" - }; struct waiting_args waiting_args = {NULL}; int ret; - ret = argp_parse(&argp, argc, argv, 0, NULL, &waiting_args); + ret = argp_parse(&waiting_argp, argc, argv, 0, NULL, &waiting_args); if (ret) return ret; @@ -151,8 +152,7 @@ static int waiting_cmd(int argc, char **argv) static void __attribute__((constructor)) waiting_ctor(void) { - cmd_register("data-waiting", "--inode --blockno ", - "print ops waiting for data blocks", waiting_cmd); + cmd_register_argp("data-waiting", &waiting_argp, GROUP_AGENT, waiting_cmd); } struct wait_err_args { @@ -298,17 +298,10 @@ static struct argp wait_err_argp = { static int wait_err_cmd(int argc, char **argv) { - struct argp argp = { - wait_err_options, - wait_err_parse_opt, - "--inode INODE-NUM --block BLOCK-NUM --version VER-NUM " - "--offset OFF-NUM --count COUNT --op OP --err ERR", - "Return error from matching waiters" - }; struct wait_err_args wait_err_args = {NULL}; int ret; - ret = argp_parse(&argp, argc, argv, 0, NULL, &wait_err_args); + ret = argp_parse(&wait_err_argp, argc, argv, 0, NULL, &wait_err_args); if (ret) return ret; @@ -318,7 +311,5 @@ static int wait_err_cmd(int argc, char **argv) static void __attribute__((constructor)) data_wait_err_ctor(void) { - cmd_register("data-wait-err", " ", - "return error from matching waiters", - wait_err_cmd); + cmd_register_argp("data-wait-err", &wait_err_argp, GROUP_AGENT, wait_err_cmd); } diff --git a/utils/src/walk_inodes.c b/utils/src/walk_inodes.c index e528991a..7dcc4377 100644 --- a/utils/src/walk_inodes.c +++ b/utils/src/walk_inodes.c @@ -187,13 +187,15 @@ static struct argp_option options[] = { { NULL } }; +static struct argp argp = { + options, + walk_inodes_parse_opt, + " FIRST-ENTRY LAST-ENTRY", + "Print range of indexed inodes" +}; + static int walk_inodes_cmd(int argc, char **argv) { - struct argp argp = { - options, - walk_inodes_parse_opt, - " FIRST-ENTRY LAST-ENTRY" - }; struct walk_inodes_args walk_inodes_args = {NULL}; int ret; @@ -207,6 +209,5 @@ static int walk_inodes_cmd(int argc, char **argv) static void __attribute__((constructor)) walk_inodes_ctor(void) { - cmd_register("walk-inodes", " ", - "print range of indexed inodes", walk_inodes_cmd); + cmd_register_argp("walk-inodes", &argp, GROUP_SEARCH, walk_inodes_cmd); }