opt: minor API tweaks.

Don't allow NULL descs, and put them in macro.  Users can use "", but make
sure it's explicit, not an accidental omission.
Use errx() instead of assert() for API usage errors.
Rename opt_flags to opt_type.

ccan/opt/opt.c

@@ -32,7 +32,7 @@ static const char *next_name(const char *names, unsigned *len)
 static const char *first_opt(unsigned *i, unsigned *len)
 {
 	for (*i = 0; *i < opt_count; (*i)++) {
-		if (opt_table[*i].flags == OPT_SUBTABLE)
+		if (opt_table[*i].type == OPT_SUBTABLE)
 			continue;
 		return first_name(opt_table[*i].names, len);
 	}
@@ -42,7 +42,7 @@ static const char *first_opt(unsigned *i, unsigned *len)
 static const char *next_opt(const char *p, unsigned *i, unsigned *len)
 {
 	for (; *i < opt_count; (*i)++) {
-		if (opt_table[*i].flags == OPT_SUBTABLE)
+		if (opt_table[*i].type == OPT_SUBTABLE)
 			continue;
 		if (!p)
 			return first_name(opt_table[*i].names, len);
@@ -107,26 +107,44 @@ static void check_opt(const struct opt_table *entry)
 	const char *p;
 	unsigned len;
 
-	assert(entry->flags == OPT_HASARG || entry->flags == OPT_NOARG);
+	if (entry->type != OPT_HASARG && entry->type != OPT_NOARG)
+		errx(1, "Option %s: unknown entry type %u",
+		     entry->names, entry->type);
+
+	if (!entry->desc)
+		errx(1, "Option %s: description cannot be NULL", entry->names);
+
+
+	if (entry->names[0] != '-')
+		errx(1, "Option %s: does not begin with '-'", entry->names);
 
-	assert(entry->names[0] == '-');
 	for (p = first_name(entry->names, &len); p; p = next_name(p, &len)) {
 		if (*p == '-') {
-			assert(len > 1);
+			if (len == 1)
+				errx(1, "Option %s: invalid long option '--'",
+				     entry->names);
 			opt_num_long++;
 		} else {
-			assert(len == 1);
-			assert(*p != ':');
+			if (len != 1)
+				errx(1, "Option %s: invalid short option"
+				     " '%.*s'", entry->names, len+1, p-1);
+			if (*p == ':')
+				errx(1, "Option %s: invalid short option '-:'",
+				     entry->names);
 			opt_num_short++;
-			if (entry->flags == OPT_HASARG) {
+			if (entry->type == OPT_HASARG) {
 				opt_num_short_arg++;
-				/* FIXME: -? with ops breaks getopt_long */
-				assert(*p != '?');
+				if (*p == '?')
+					errx(1, "Option %s: '-?' cannot take"
+					     " an argument", entry->names);
 			}
 		}
 		/* Don't document args unless there are some. */
-		if (entry->flags == OPT_NOARG)
-			assert(p[len] != ' ' && p[len] != '=');
+		if (entry->type == OPT_NOARG) {
+			if (p[len] == ' ' || p[len] == '=')
+				errx(1, "Option %s: does not take arguments"
+				     "'%s'", entry->names, p+len+1);
+		}
 	}
 }
 
@@ -136,7 +154,7 @@ static void add_opt(const struct opt_table *entry)
 	opt_table[opt_count++] = *entry;
 }
 
-void _opt_register(const char *names, enum opt_flags flags,
+void _opt_register(const char *names, enum opt_type type,
 		   char *(*cb)(void *arg),
 		   char *(*cb_arg)(const char *optarg, void *arg),
 		   void (*show)(char buf[OPT_SHOW_LEN], const void *arg),
@@ -144,7 +162,7 @@ void _opt_register(const char *names, enum opt_flags flags,
 {
 	struct opt_table opt;
 	opt.names = names;
-	opt.flags = flags;
+	opt.type = type;
 	opt.cb = cb;
 	opt.cb_arg = cb_arg;
 	opt.show = show;
@@ -162,8 +180,8 @@ void opt_register_table(const struct opt_table entry[], const char *desc)
 		struct opt_table heading = OPT_SUBTABLE(NULL, desc);
 		add_opt(&heading);
 	}
-	for (i = 0; entry[i].flags != OPT_END; i++) {
-		if (entry[i].flags == OPT_SUBTABLE)
+	for (i = 0; entry[i].type != OPT_END; i++) {
+		if (entry[i].type == OPT_SUBTABLE)
 			opt_register_table(subtable_of(&entry[i]),
 					   entry[i].desc);
 		else {
@@ -186,7 +204,7 @@ static char *make_optstring(void)
 	str[num++] = ':';
 	for (p = first_sopt(&i); p; p = next_sopt(p, &i)) {
 		str[num++] = *p;
-		if (opt_table[i].flags == OPT_HASARG)
+		if (opt_table[i].type == OPT_HASARG)
 			str[num++] = ':';
 	}
 	str[num++] = '\0';
@@ -205,7 +223,7 @@ static struct option *make_options(void)
 		memcpy(buf, p, len);
 		buf[len] = 0;
 		options[num].name = buf;
-		options[num].has_arg = (opt_table[i].flags == OPT_HASARG);
+		options[num].has_arg = (opt_table[i].type == OPT_HASARG);
 		options[num].flag = NULL;
 		options[num].val = 0;
 		num++;
@@ -300,7 +318,7 @@ bool opt_parse(int *argc, char *argv[], void (*errlog)(const char *fmt, ...))
 		else
 			e = find_long(longidx, &name);
 
-		if (e->flags == OPT_HASARG)
+		if (e->type == OPT_HASARG)
 			problem = e->cb_arg(optarg, e->arg);
 		else
 			problem = e->cb(e->arg);

ccan/opt/opt.h

@@ -5,7 +5,7 @@
 
 /* You can use this directly to build tables, but the macros will ensure
  * consistency and type safety. */
-enum opt_flags {
+enum opt_type {
 	OPT_NOARG = 1,		/* -f/--foo */
 	OPT_HASARG = 2,		/* -f arg/--foo=arg/--foo arg */
 	OPT_SUBTABLE = 4,	/* Actually, longopt points to a subtable... */
@@ -17,7 +17,7 @@ enum opt_flags {
 
 struct opt_table {
 	const char *names; /* slash-separated names, --longopt or -s */
-	enum opt_flags flags;
+	enum opt_type type;
 	char *(*cb)(void *arg); /* OPT_NOARG */
 	char *(*cb_arg)(const char *optarg, void *arg); /* OPT_HASARG */
 	void (*show)(char buf[OPT_SHOW_LEN], const void *arg);
@@ -30,6 +30,7 @@ struct opt_table {
  * @names: the names of the option eg. "--foo", "-f" or "--foo/-f/--foobar".
  * @cb: the callback when the option is found.
  * @arg: the argument to hand to @cb.
+ * @desc: the description for opt_usage(), or opt_hidden.
  *
  * This is a typesafe wrapper for intializing a struct opt_table.  The callback
  * of type "char *cb(type *)", "char *cb(const type *)" or "char *cb(void *)",
@@ -46,8 +47,8 @@ struct opt_table {
  * See Also:
  *	OPT_WITH_ARG()
  */
-#define OPT_WITHOUT_ARG(names, cb, arg) \
-	(names), OPT_CB_NOARG((cb), (arg))
+#define OPT_WITHOUT_ARG(names, cb, arg, desc)	\
+	(names), OPT_CB_NOARG((cb), (arg)), (desc)
 
 /**
  * OPT_WITH_ARG() - macro for initializing long and short option (with arg)
@@ -55,6 +56,7 @@ struct opt_table {
  * @cb: the callback when the option is found (along with <arg>).
  * @show: the callback to print the value in get_usage (or NULL)
  * @arg: the argument to hand to @cb and @show
+ * @desc: the description for opt_usage(), or opt_hidden.
  *
  * This is a typesafe wrapper for intializing a struct opt_table.  The callback
  * is of type "char *cb(const char *, type *)",
@@ -80,8 +82,8 @@ struct opt_table {
  * See Also:
  *	OPT_WITHOUT_ARG()
  */
-#define OPT_WITH_ARG(name, cb, show, arg) \
-	(name), OPT_CB_ARG((cb), (show), (arg))
+#define OPT_WITH_ARG(name, cb, show, arg, desc)	\
+	(name), OPT_CB_ARG((cb), (show), (arg)), (desc)
 
 /**
  * OPT_SUBTABLE() - macro for including another table inside a table.
@@ -286,7 +288,7 @@ char *opt_usage_and_exit(const char *extra);
 	(arg)
 
 /* Non-typesafe register function. */
-void _opt_register(const char *names, enum opt_flags flags,
+void _opt_register(const char *names, enum opt_type type,
 		   char *(*cb)(void *arg),
 		   char *(*cb_arg)(const char *optarg, void *arg),
 		   void (*show)(char buf[OPT_SHOW_LEN], const void *arg),

ccan/opt/test/run-correct-reporting.c

@@ -11,7 +11,7 @@ int main(int argc, char *argv[])
 	plan_tests(12);
 
 	/* --aaa without args. */
-	opt_register_arg("-a/--aaa", test_arg, NULL, "aaa", NULL);
+	opt_register_arg("-a/--aaa", test_arg, NULL, "aaa", "");
 	ok1(!parse_args(&argc, &argv, "--aaa", NULL));
 	ok1(strstr(err_output, ": --aaa: option requires an argument"));
 	free(err_output);
@@ -22,7 +22,7 @@ int main(int argc, char *argv[])
 	err_output = NULL;
 
 	/* Multiple */
-	opt_register_arg("--bbb/-b/-c/--ccc", test_arg, NULL, "aaa", NULL);
+	opt_register_arg("--bbb/-b/-c/--ccc", test_arg, NULL, "aaa", "");
 	ok1(!parse_args(&argc, &argv, "--bbb", NULL));
 	ok1(strstr(err_output, ": --bbb: option requires an argument"));
 	free(err_output);

ccan/opt/test/run-helpers.c

@@ -55,10 +55,10 @@ int main(int argc, char *argv[])
 	{
 		bool arg = false;
 		reset_options();
-		opt_register_noarg("-a", opt_set_bool, &arg, NULL);
+		opt_register_noarg("-a", opt_set_bool, &arg, "");
 		ok1(parse_args(&argc, &argv, "-a", NULL));
 		ok1(arg);
-		opt_register_arg("-b", opt_set_bool_arg, NULL, &arg, NULL);
+		opt_register_arg("-b", opt_set_bool_arg, NULL, &arg, "");
 		ok1(parse_args(&argc, &argv, "-b", "no", NULL));
 		ok1(!arg);
 		ok1(parse_args(&argc, &argv, "-b", "yes", NULL));
@@ -75,11 +75,11 @@ int main(int argc, char *argv[])
 	{
 		bool arg = true;
 		reset_options();
-		opt_register_noarg("-a", opt_set_invbool, &arg, NULL);
+		opt_register_noarg("-a", opt_set_invbool, &arg, "");
 		ok1(parse_args(&argc, &argv, "-a", NULL));
 		ok1(!arg);
 		opt_register_arg("-b", opt_set_invbool_arg, NULL,
-				 &arg, NULL);
+				 &arg, "");
 		ok1(parse_args(&argc, &argv, "-b", "no", NULL));
 		ok1(arg);
 		ok1(parse_args(&argc, &argv, "-b", "yes", NULL));
@@ -96,7 +96,7 @@ int main(int argc, char *argv[])
 	{
 		char *arg = (char *)"wrong";
 		reset_options();
-		opt_register_arg("-a", opt_set_charp, NULL, &arg, NULL);
+		opt_register_arg("-a", opt_set_charp, NULL, &arg, "All");
 		ok1(parse_args(&argc, &argv, "-a", "string", NULL));
 		ok1(strcmp(arg, "string") == 0);
 	}
@@ -104,7 +104,7 @@ int main(int argc, char *argv[])
 	{
 		int arg = 1000;
 		reset_options();
-		opt_register_arg("-a", opt_set_intval, NULL, &arg, NULL);
+		opt_register_arg("-a", opt_set_intval, NULL, &arg, "All");
 		ok1(parse_args(&argc, &argv, "-a", "9999", NULL));
 		ok1(arg == 9999);
 		ok1(parse_args(&argc, &argv, "-a", "-9999", NULL));
@@ -121,7 +121,7 @@ int main(int argc, char *argv[])
 	{
 		unsigned int arg = 1000;
 		reset_options();
-		opt_register_arg("-a", opt_set_uintval, NULL, &arg, NULL);
+		opt_register_arg("-a", opt_set_uintval, NULL, &arg, "All");
 		ok1(parse_args(&argc, &argv, "-a", "9999", NULL));
 		ok1(arg == 9999);
 		ok1(!parse_args(&argc, &argv, "-a", "-9999", NULL));
@@ -145,7 +145,7 @@ int main(int argc, char *argv[])
 	{
 		long int arg = 1000;
 		reset_options();
-		opt_register_arg("-a", opt_set_longval, NULL, &arg, NULL);
+		opt_register_arg("-a", opt_set_longval, NULL, &arg, "All");
 		ok1(parse_args(&argc, &argv, "-a", "9999", NULL));
 		ok1(arg == 9999);
 		ok1(parse_args(&argc, &argv, "-a", "-9999", NULL));
@@ -164,7 +164,7 @@ int main(int argc, char *argv[])
 	{
 		unsigned long int arg = 1000;
 		reset_options();
-		opt_register_arg("-a", opt_set_ulongval, NULL, &arg, NULL);
+		opt_register_arg("-a", opt_set_ulongval, NULL, &arg, "All");
 		ok1(parse_args(&argc, &argv, "-a", "9999", NULL));
 		ok1(arg == 9999);
 		ok1(!parse_args(&argc, &argv, "-a", "-9999", NULL));
@@ -182,7 +182,7 @@ int main(int argc, char *argv[])
 	{
 		int arg = 1000;
 		reset_options();
-		opt_register_noarg("-a", opt_inc_intval, &arg, NULL);
+		opt_register_noarg("-a", opt_inc_intval, &arg, "");
 		ok1(parse_args(&argc, &argv, "-a", NULL));
 		ok1(arg == 1001);
 		ok1(parse_args(&argc, &argv, "-a", "-a", NULL));
@@ -196,7 +196,7 @@ int main(int argc, char *argv[])
 		int exitval;
 		reset_options();
 		opt_register_noarg("-a",
-				   opt_version_and_exit, "1.2.3", NULL);
+				   opt_version_and_exit, "1.2.3", "");
 		exitval = setjmp(exited);
 		if (exitval == 0) {
 			parse_args(&argc, &argv, "-a", NULL);
@@ -214,7 +214,7 @@ int main(int argc, char *argv[])
 		int exitval;
 		reset_options();
 		opt_register_noarg("-a",
-				   opt_usage_and_exit, "[args]", NULL);
+				   opt_usage_and_exit, "[args]", "");
 		exitval = setjmp(exited);
 		if (exitval == 0) {
 			parse_args(&argc, &argv, "-a", NULL);

ccan/opt/test/run.c

@@ -20,7 +20,7 @@ int main(int argc, char *argv[])
 	plan_tests(148);
 
 	/* Simple short arg.*/
-	opt_register_noarg("-a", test_noarg, NULL, NULL);
+	opt_register_noarg("-a", test_noarg, NULL, "All");
 	ok1(parse_args(&argc, &argv, "-a", NULL));
 	ok1(argc == 1);
 	ok1(argv[0] == myname);
@@ -28,7 +28,7 @@ int main(int argc, char *argv[])
 	ok1(test_cb_called == 1);
 
 	/* Simple long arg. */
-	opt_register_noarg("--aaa", test_noarg, NULL, NULL);
+	opt_register_noarg("--aaa", test_noarg, NULL, "AAAAll");
 	ok1(parse_args(&argc, &argv, "--aaa", NULL));
 	ok1(argc == 1);
 	ok1(argv[0] == myname);
@@ -36,7 +36,7 @@ int main(int argc, char *argv[])
 	ok1(test_cb_called == 2);
 
 	/* Both long and short args. */
-	opt_register_noarg("--aaa/-a", test_noarg, NULL, NULL);
+	opt_register_noarg("--aaa/-a", test_noarg, NULL, "AAAAAAll");
 	ok1(parse_args(&argc, &argv, "--aaa", "-a", NULL));
 	ok1(argc == 1);
 	ok1(argv[0] == myname);
@@ -54,7 +54,7 @@ int main(int argc, char *argv[])
 	/* Argument variants. */
 	reset_options();
 	test_cb_called = 0;
-	opt_register_arg("-a/--aaa", test_arg, NULL, "aaa", NULL);
+	opt_register_arg("-a/--aaa", test_arg, NULL, "aaa", "AAAAAAll");
 	ok1(parse_args(&argc, &argv, "--aaa", "aaa", NULL));
 	ok1(argc == 1);
 	ok1(argv[0] == myname);
@@ -201,7 +201,7 @@ int main(int argc, char *argv[])
 	reset_options();
 
 	/* glibc's getopt does not handle ? with arguments. */
-	opt_register_noarg("-?", test_noarg, NULL, NULL);
+	opt_register_noarg("-?", test_noarg, NULL, "Help");
 	ok1(parse_args(&argc, &argv, "-?", NULL));
 	ok1(test_cb_called == 1);
 	ok1(parse_args(&argc, &argv, "-a", NULL) == false);

ccan/opt/test/utils.c

@@ -70,33 +70,33 @@ bool parse_args(int *argc, char ***argv, ...)
 
 struct opt_table short_table[] = {
 	/* Short opts, different args. */
-	{ OPT_WITHOUT_ARG("-a", test_noarg, "a"), "Description of a" },
-	{ OPT_WITH_ARG("-b", test_arg, show_arg, "b"), "Description of b" },
+	{ OPT_WITHOUT_ARG("-a", test_noarg, "a", "Description of a") },
+	{ OPT_WITH_ARG("-b", test_arg, show_arg, "b", "Description of b") },
 	OPT_ENDTABLE
 };
 
 struct opt_table long_table[] = {
 	/* Long opts, different args. */
-	{ OPT_WITHOUT_ARG("--ddd", test_noarg, "ddd"), "Description of ddd" },
-	{ OPT_WITH_ARG("--eee <filename>", test_arg, show_arg, "eee"), },
+	{ OPT_WITHOUT_ARG("--ddd", test_noarg, "ddd", "Description of ddd") },
+	{ OPT_WITH_ARG("--eee <filename>", test_arg, show_arg, "eee", "") },
 	OPT_ENDTABLE
 };
 
 struct opt_table long_and_short_table[] = {
 	/* Short and long, different args. */
-	{ OPT_WITHOUT_ARG("--ggg/-g", test_noarg, "ggg"),
-	  "Description of ggg" },
-	{ OPT_WITH_ARG("-h/--hhh", test_arg, NULL, "hhh"),
-	  "Description of hhh"},
+	{ OPT_WITHOUT_ARG("--ggg/-g", test_noarg, "ggg",
+			  "Description of ggg") },
+	{ OPT_WITH_ARG("-h/--hhh", test_arg, NULL, "hhh",
+		       "Description of hhh") },
 	OPT_ENDTABLE
 };
 
 /* Sub-table test. */
 struct opt_table subtables[] = {
 	/* Two short, and two long long, no description */
-	{ OPT_WITH_ARG("--jjj/-j/--lll/-l", test_arg, show_arg, "jjj") },
+	{ OPT_WITH_ARG("--jjj/-j/--lll/-l", test_arg, show_arg, "jjj", "") },
 	/* Hidden option */
-	{ OPT_WITH_ARG("--mmm/-m", test_arg, show_arg, "mmm"), opt_hidden },
+	{ OPT_WITH_ARG("--mmm/-m", test_arg, show_arg, "mmm", opt_hidden) },
 	OPT_SUBTABLE(short_table, NULL),
 	OPT_SUBTABLE(long_table, "long table options"),
 	OPT_SUBTABLE(long_and_short_table, NULL),

ccan/opt/usage.c

@@ -35,15 +35,13 @@ char *opt_usage(const char *argv0, const char *extra)
 		+ strlen("\n");
 
 	for (i = 0; i < opt_count; i++) {
-		if (opt_table[i].flags == OPT_SUBTABLE) {
+		if (opt_table[i].type == OPT_SUBTABLE) {
 			len += strlen("\n") + strlen(opt_table[i].desc)
 				+ strlen(":\n");
 		} else if (opt_table[i].desc != opt_hidden) {
 			len += strlen(opt_table[i].names) + strlen(" <arg>");
-			if (opt_table[i].desc) {
-				len += strlen(OPT_SPACE_PAD)
-					+ strlen(opt_table[i].desc) + 1;
-			}
+			len += strlen(OPT_SPACE_PAD)
+				+ strlen(opt_table[i].desc) + 1;
 			if (opt_table[i].show) {
 				len += strlen("(default: %s)")
 					+ OPT_SHOW_LEN + sizeof("...");
@@ -73,29 +71,26 @@ char *opt_usage(const char *argv0, const char *extra)
 	for (i = 0; i < opt_count; i++) {
 		if (opt_table[i].desc == opt_hidden)
 			continue;
-		if (opt_table[i].flags == OPT_SUBTABLE) {
+		if (opt_table[i].type == OPT_SUBTABLE) {
 			p += sprintf(p, "%s:\n", opt_table[i].desc);
 			continue;
 		}
 		len = sprintf(p, "%s", opt_table[i].names);
-		if (opt_table[i].flags == OPT_HASARG
+		if (opt_table[i].type == OPT_HASARG
 		    && !strchr(opt_table[i].names, ' ')
 		    && !strchr(opt_table[i].names, '='))
 			len += sprintf(p + len, " <arg>");
-		if (opt_table[i].desc || opt_table[i].show)
-			len += sprintf(p + len, "%.*s",
-				       len < strlen(OPT_SPACE_PAD)
-				       ? strlen(OPT_SPACE_PAD) - len : 1,
-				       OPT_SPACE_PAD);
+		len += sprintf(p + len, "%.*s",
+			       len < strlen(OPT_SPACE_PAD)
+			       ? strlen(OPT_SPACE_PAD) - len : 1,
+			       OPT_SPACE_PAD);
 
-		if (opt_table[i].desc)
-			len += sprintf(p + len, "%s", opt_table[i].desc);
+		len += sprintf(p + len, "%s", opt_table[i].desc);
 		if (opt_table[i].show) {
 			char buf[OPT_SHOW_LEN + sizeof("...")];
 			strcpy(buf + OPT_SHOW_LEN, "...");
 			opt_table[i].show(buf, opt_table[i].arg);
-			len += sprintf(p + len, "%s(default: %s)",
-				       opt_table[i].desc ? " " : "", buf);
+			len += sprintf(p + len, " (default: %s)", buf);
 		}
 		p += len;
 		p += sprintf(p, "\n");