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.

diff --git a/ccan/opt/opt.c b/ccan/opt/opt.c
index e7ebc3dd0550e2181b13c41438ddeb9e8f244dff..841e9b4eb2d4021c6b97432e2c65edb8e8754d90 100644 (file)
--- a/ccan/opt/opt.c
+++ b/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);

diff --git a/ccan/opt/opt.h b/ccan/opt/opt.h
index 65008091a0861ade804d06e8fc302bcf74a6aa51..1e25cce370ad19b002587a2d59951ddd0236bd5b 100644 (file)
--- a/ccan/opt/opt.h
+++ b/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),

diff --git a/ccan/opt/test/run-correct-reporting.c b/ccan/opt/test/run-correct-reporting.c
index 44f68ae2034b01d7146e6ac50a80f72346dbfbb2..bbac3402dde35a70d12ff58dd1961377a791e5b3 100644 (file)
--- a/ccan/opt/test/run-correct-reporting.c
+++ b/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);

diff --git a/ccan/opt/test/run-helpers.c b/ccan/opt/test/run-helpers.c
index 48b7cd8b11dfba607bdc32a501bde99f37370487..91e66374bbaf42e7e688521165cc93c343426ed5 100644 (file)
--- a/ccan/opt/test/run-helpers.c
+++ b/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);

diff --git a/ccan/opt/test/run.c b/ccan/opt/test/run.c
index 4eb78c936a44eeb4cdcad26a634c4f4dd3c52615..82c0398c92c43d5761df1ee29f63fa8d9338e187 100644 (file)
--- a/ccan/opt/test/run.c
+++ b/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);

diff --git a/ccan/opt/test/utils.c b/ccan/opt/test/utils.c
index bafb67438247a6d381747731231fefaa3d9b6b4b..4e5b4d77a310c1b2bf9476d43f1de6be4d226b56 100644 (file)
--- a/ccan/opt/test/utils.c
+++ b/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),

diff --git a/ccan/opt/usage.c b/ccan/opt/usage.c
index 25678d662d7feee7120b3c21559bf10461ea41de..b7381bc7378c6673c742d6dc23b0c42f211e8265 100644 (file)
--- a/ccan/opt/usage.c
+++ b/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");