sss tools: improve option handling

The crash describe by ticket #2802 is caused by providing NULL options
in popt and yet trying to iterate over them. Instead of simply testing
for NULL this patch creates a new option table table merges several
option tables together, thus improving and simplifying usage string.

Resolves:
https://fedorahosted.org/sssd/ticket/2802

Reviewed-by: Pavel Reichl <preichl@redhat.com>

2 files changed, 30 insertions, 20 deletions

diff --git a/src/tools/common/sss_tools.c b/src/tools/common/sss_tools.c
index d50e9af7d..0ada62e3e 100644
--- a/src/tools/common/sss_tools.c
+++ b/src/tools/common/sss_tools.c
@@ -43,6 +43,19 @@ static void sss_tool_print_common_opts(void)
                     _("Enable debug at level"));
 }
 
+static struct poptOption *sss_tool_common_opts_table(void)
+{
+    static struct poptOption common_opts[] = {
+        {"debug", '\0', POPT_ARG_INT, NULL,
+            0, NULL, NULL },
+        POPT_TABLEEND
+    };
+
+    common_opts[0].descrip = _("The debug level to run with");
+
+    return common_opts;
+}
+
 static void sss_tool_common_opts(struct sss_tool_ctx *tool_ctx,
                                  int *argc, const char **argv)
 {
@@ -247,13 +260,6 @@ int sss_tool_route(int argc, const char **argv,
     return sss_tool_usage(argv[0], commands);
 }
 
-static void sss_tool_popt_print_help(poptContext pc)
-{
-    poptPrintHelp(pc, stderr, 0);
-    fprintf(stderr, "\n");
-    sss_tool_print_common_opts();
-}
-
 int sss_tool_popt_ex(struct sss_cmdline *cmdline,
                      struct poptOption *options,
                      enum sss_tool_opt require_option,
@@ -263,20 +269,26 @@ int sss_tool_popt_ex(struct sss_cmdline *cmdline,
                      const char *fopt_help,
                      const char **_fopt)
 {
-    const char *optstr;
+    struct poptOption opts_table[] = {
+        {NULL, '\0', POPT_ARG_INCLUDE_TABLE, options, \
+         0, _("Command options:"), NULL },
+        {NULL, '\0', POPT_ARG_INCLUDE_TABLE, sss_tool_common_opts_table(), \
+         0, _("Common options:"), NULL },
+        POPT_AUTOHELP
+        POPT_TABLEEND
+    };
     char *help;
     poptContext pc;
     int ret;
 
     /* Create help option string. We always need to append command name since
      * we use POPT_CONTEXT_KEEP_FIRST. */
-    optstr = options == NULL ? "" : _(" [OPTIONS...]");
     if (fopt_name == NULL) {
-        help = talloc_asprintf(NULL, "%s %s%s",
-                               cmdline->exec, cmdline->command, optstr);
+        help = talloc_asprintf(NULL, "%s %s %s", cmdline->exec,
+                               cmdline->command, _("[OPTIONS...]"));
     } else {
-        help = talloc_asprintf(NULL, "%s %s %s%s",
-                           cmdline->exec, cmdline->command, fopt_name, optstr);
+        help = talloc_asprintf(NULL, "%s %s %s %s", cmdline->exec,
+                               cmdline->command, fopt_name, _("[OPTIONS...]"));
     }
     if (help == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, "talloc_asprintf() failed\n");
@@ -287,7 +299,7 @@ int sss_tool_popt_ex(struct sss_cmdline *cmdline,
      * command argv which does not contain executable (argv[0]), therefore
      * we need to use KEEP_FIRST that ensures argv[0] is also processed. */
     pc = poptGetContext(cmdline->exec, cmdline->argc, cmdline->argv,
-                        options, POPT_CONTEXT_KEEP_FIRST);
+                        opts_table, POPT_CONTEXT_KEEP_FIRST);
 
     poptSetOtherOptionHelp(pc, help);
 
@@ -303,7 +315,7 @@ int sss_tool_popt_ex(struct sss_cmdline *cmdline,
         } else {
             fprintf(stderr, _("Invalid option %s: %s\n\n"),
                     poptBadOption(pc, 0), poptStrerror(ret));
-            sss_tool_popt_print_help(pc);
+            poptPrintHelp(pc, stderr, 0);
             ret = EXIT_FAILURE;
             goto done;
         }
@@ -314,7 +326,7 @@ int sss_tool_popt_ex(struct sss_cmdline *cmdline,
         *_fopt = poptGetArg(pc);
         if (*_fopt == NULL) {
             fprintf(stderr, _("Missing option: %s\n\n"), fopt_help);
-            sss_tool_popt_print_help(pc);
+            poptPrintHelp(pc, stderr, 0);
             ret = EXIT_FAILURE;
             goto done;
         }
@@ -322,7 +334,7 @@ int sss_tool_popt_ex(struct sss_cmdline *cmdline,
         /* No more arguments expected. If something follows it is an error. */
         if (poptGetArg(pc)) {
             fprintf(stderr, _("Only one free argument is expected!\n\n"));
-            sss_tool_popt_print_help(pc);
+            poptPrintHelp(pc, stderr, 0);
             ret = EXIT_FAILURE;
             goto done;
         }
@@ -332,7 +344,7 @@ int sss_tool_popt_ex(struct sss_cmdline *cmdline,
     if (require_option == SSS_TOOL_OPT_REQUIRED
             && ((_fopt != NULL && cmdline->argc < 2) || cmdline->argc < 1)) {
         fprintf(stderr, _("At least one option is required!\n\n"));
-        sss_tool_popt_print_help(pc);
+        poptPrintHelp(pc, stderr, 0);
         ret = EXIT_FAILURE;
         goto done;
     }
diff --git a/src/tools/sss_override.c b/src/tools/sss_override.c
index 0d7a46906..075f024f0 100644
--- a/src/tools/sss_override.c
+++ b/src/tools/sss_override.c
@@ -92,7 +92,6 @@ static int parse_cmdline_user_add(struct sss_cmdline *cmdline,
                                   struct override_user *user)
 {
     struct poptOption options[] = {
-        POPT_AUTOHELP
         {"name", 'n', POPT_ARG_STRING, &user->name, 0, _("Override name"), NULL },
         {"uid", 'u', POPT_ARG_INT, &user->uid, 0, _("Override uid (non-zero value)"), NULL },
         {"gid", 'g', POPT_ARG_INT, &user->gid, 0, _("Override gid (non-zero value)"), NULL },
@@ -119,7 +118,6 @@ static int parse_cmdline_group_add(struct sss_cmdline *cmdline,
                                    struct override_group *group)
 {
     struct poptOption options[] = {
-        POPT_AUTOHELP
         {"name", 'n', POPT_ARG_STRING, &group->name, 0, _("Override name"), NULL },
         {"gid", 'g', POPT_ARG_INT, &group->gid, 0, _("Override gid"), NULL },
         POPT_TABLEEND