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 <agrover@versity.com>

utils/src/cmd.c

@@ -4,35 +4,38 @@
 #include <stdbool.h>
 #include <string.h>
 #include <assert.h>
+#include <argp.h>
 
 #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 <command> [<args>]\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 <command> [<args>]\n\n");
+	fprintf(stderr, "Selected fs defaults to current working directory.\n");
+	fprintf(stderr, "See <command> --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) {

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);

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] <sysfs dir>",
-		     "show [tabular] counters for a given mounted volume",
-		     counters_cmd);
+	cmd_register_argp("counters", &argp, GROUP_INFO, counters_cmd);
 }

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", "<path>",
-		     "Show metadata and data block usage", df_cmd);
+	cmd_register_argp("df", &argp, GROUP_CORE, df_cmd);
 }

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", "<ino> <path>",
-		     "print paths that refer to inode #", ino_path_cmd);
+	cmd_register_argp("ino-path", &argp, GROUP_SEARCH, ino_path_cmd);
 }

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", "<path>",
-		     "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);
 }

utils/src/main.c

@@ -4,6 +4,7 @@
 #include <stdbool.h>
 #include <string.h>
 #include <assert.h>
+#include <argp.h>
 
 #include "cmd.h"
 #include "util.h"

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", "<meta-device> <data-device>", "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);

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", "<device>", "print metadata structures",
-			print_cmd);
+	cmd_register_argp("print", &argp, GROUP_DEBUG, print_cmd);
 }

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 <path>",
-		     "print inode numbers of inodes which may have given xattr",
-		     search_xattrs_cmd);
+	cmd_register_argp("search-xattrs", &argp, GROUP_INFO, search_xattrs_cmd);
 }

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", "<path>",
-		     "set attributes on newly-created zero-length file",
-		     setattr_cmd);
+	cmd_register_argp("setattr", &argp, GROUP_AGENT, setattr_cmd);
 }

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", "<archive file> <file> -V <version>",
-		     "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", "<path> <vers>",
-		     "mark file region offline and free extents", release_cmd);
+	cmd_register_argp("release", &release_argp, GROUP_AGENT, release_cmd);
 }

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", "<path>",
-		     "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", "<path>",
-		     "show scoutfs file system information", statfs_more_cmd);
+	cmd_register_argp("statfs", &statfs_argp, GROUP_INFO, statfs_more_cmd);
 }

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);
 

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 <inode> --blockno <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", "<path> <ino> <vers> <offset> <count> <op> <err>",
-		     "return error from matching waiters",
-		     wait_err_cmd);
+	cmd_register_argp("data-wait-err", &wait_err_argp, GROUP_AGENT, wait_err_cmd);
 }

utils/src/walk_inodes.c

@@ -187,13 +187,15 @@ static struct argp_option options[] = {
 	{ NULL }
 };
 
+static struct argp argp = {
+	options,
+	walk_inodes_parse_opt,
+	"<meta_seq|data_seq> 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,
-		"<meta_seq|data_seq> 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", "<index> <first> <last>",
-		     "print range of indexed inodes", walk_inodes_cmd);
+	cmd_register_argp("walk-inodes", &argp, GROUP_SEARCH, walk_inodes_cmd);
 }