Don't spawn man for --help

Convert pkaction and pkttyagent to use GOptionContext.

Don't convert pkcheck and only add --help output text because its
non-standard --details(which requires two arguments) can't be
implemented using GOptionContext.

Don't touch pkexec, in a (futile?) attempt to minimize the amount
of complex code running before authentication.

This leaves the option processing lax as it was (e.g. accepting
contradicting options, ignoring non-option arguments), and should only
affect the handling of --help and behavior when invalid arguments are
detected.

https://bugs.freedesktop.org/show_bug.cgi?id=29936

4 files changed, 141 insertions, 186 deletions

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 6e76bdd..ed9faa5 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -3,4 +3,7 @@
 [encoding: UTF-8]
 actions/org.freedesktop.policykit.policy.in
 src/examples/org.freedesktop.policykit.examples.pkexec.policy.in
+src/programs/pkaction.c
+src/programs/pkcheck.c
 src/programs/pkexec.c
+src/programs/pkttyagent.c
diff --git a/src/programs/pkaction.c b/src/programs/pkaction.c
index 00b2c57..f2969cc 100644
--- a/src/programs/pkaction.c
+++ b/src/programs/pkaction.c
@@ -24,26 +24,10 @@
 #endif
 
 #include <stdio.h>
+#include <glib/gi18n.h>
 #include <polkit/polkit.h>
 
 static void
-usage (int argc, char *argv[])
-{
-  GError *error;
-
-  error = NULL;
-  if (!g_spawn_command_line_sync ("man pkaction",
-                                  NULL,
-                                  NULL,
-                                  NULL,
-                                  &error))
-    {
-      g_printerr ("Cannot show manual page: %s\n", error->message);
-      g_error_free (error);
-    }
-}
-
-static void
 print_action (PolkitActionDescription *action,
               gboolean                 opt_verbose)
 {
@@ -104,68 +88,59 @@ action_desc_compare_by_action_id_func (PolkitActionDescription *a,
 int
 main (int argc, char *argv[])
 {
-  guint n;
   guint ret;
-  gchar *action_id;
-  gboolean opt_show_help;
+  gchar *opt_action_id;
   gboolean opt_show_version;
   gboolean opt_verbose;
+  GOptionEntry options[] =
+    {
+      {
+	"action-id", 'a', 0, G_OPTION_ARG_STRING, &opt_action_id,
+	N_("Only output information about ACTION"), N_("ACTION")
+      },
+      {
+	"verbose", 'v', 0, G_OPTION_ARG_NONE, &opt_verbose,
+	N_("Output detailed action information"), NULL
+      },
+      {
+	"version", 0, 0, G_OPTION_ARG_NONE, &opt_show_version,
+	N_("Show version"), NULL
+      },
+      { NULL, 0, 0, 0, NULL, NULL, NULL }
+    };
+  GOptionContext *context;
   PolkitAuthority *authority;
   GList *l;
   GList *actions;
   GError *error;
 
-  action_id = NULL;
+  opt_action_id = NULL;
+  context = NULL;
   authority = NULL;
   actions = NULL;
   ret = 1;
 
   g_type_init ();
 
-  opt_show_help = FALSE;
   opt_show_version = FALSE;
   opt_verbose = FALSE;
-  for (n = 1; n < (guint) argc; n++)
-    {
-      if (g_strcmp0 (argv[n], "--help") == 0)
-        {
-          opt_show_help = TRUE;
-        }
-      else if (g_strcmp0 (argv[n], "--version") == 0)
-        {
-          opt_show_version = TRUE;
-        }
-      else if (g_strcmp0 (argv[n], "--action-id") == 0 || g_strcmp0 (argv[n], "-a") == 0)
-        {
-          n++;
-          if (n >= (guint) argc)
-            {
-              usage (argc, argv);
-              goto out;
-            }
-
-          action_id = g_strdup (argv[n]);
-        }
-      else if (g_strcmp0 (argv[n], "--verbose") == 0 || g_strcmp0 (argv[n], "-v") == 0)
-        {
-          opt_verbose = TRUE;
-        }
-    }
 
-  if (opt_show_help)
+  error = NULL;
+  context = g_option_context_new (N_("[--action-id ACTION]"));
+  g_option_context_add_main_entries (context, options, GETTEXT_PACKAGE);
+  if (!g_option_context_parse (context, &argc, &argv, &error))
     {
-      usage (argc, argv);
-      ret = 0;
+      g_printerr ("%s: %s\n", g_get_prgname (), error->message);
+      g_error_free (error);
       goto out;
     }
-  else if (opt_show_version)
+  if (opt_show_version)
     {
       g_print ("pkaction version %s\n", PACKAGE_VERSION);
       ret = 0;
       goto out;
     }
 
-  error = NULL;
   authority = polkit_authority_get_sync (NULL /* GCancellable* */, &error);
   if (authority == NULL)
     {
@@ -185,7 +160,7 @@ main (int argc, char *argv[])
       goto out;
     }
 
-  if (action_id != NULL)
+  if (opt_action_id != NULL)
     {
       for (l = actions; l != NULL; l = l->next)
         {
@@ -194,7 +169,7 @@ main (int argc, char *argv[])
 
           id = polkit_action_description_get_action_id (action);
 
-          if (g_strcmp0 (id, action_id) == 0)
+          if (g_strcmp0 (id, opt_action_id) == 0)
             {
               print_action (action, opt_verbose);
               break;
@@ -203,7 +178,7 @@ main (int argc, char *argv[])
 
       if (l == NULL)
         {
-          g_printerr ("No action with action id %s\n", action_id);
+          g_printerr ("No action with action id %s\n", opt_action_id);
           goto out;
         }
     }
@@ -224,11 +199,13 @@ main (int argc, char *argv[])
   g_list_foreach (actions, (GFunc) g_object_unref, NULL);
   g_list_free (actions);
 
-  g_free (action_id);
+  g_free (opt_action_id);
 
   if (authority != NULL)
     g_object_unref (authority);
 
+  g_option_context_free (context);
+
   return ret;
 }
 
diff --git a/src/programs/pkcheck.c b/src/programs/pkcheck.c
index 719a36c..20713cc 100644
--- a/src/programs/pkcheck.c
+++ b/src/programs/pkcheck.c
@@ -24,25 +24,31 @@
 #endif
 
 #include <stdio.h>
+#include <glib/gi18n.h>
 #include <polkit/polkit.h>
 #define POLKIT_AGENT_I_KNOW_API_IS_SUBJECT_TO_CHANGE
 #include <polkitagent/polkitagent.h>
 
 static void
-usage (int argc, char *argv[])
+help (void)
 {
-  GError *error;
-
-  error = NULL;
-  if (!g_spawn_command_line_sync ("man pkcheck",
-                                  NULL,
-                                  NULL,
-                                  NULL,
-                                  &error))
-    {
-      g_printerr ("Cannot show manual page: %s\n", error->message);
-      g_error_free (error);
-    }
+  g_print (_("Usage:\n"
+"  pkcheck [OPTION...]\n"
+"\n"
+"Help Options:\n"
+"  -h, --help                         Show help options\n"
+"\n"
+"Application Options:\n"
+"  -a, --action-id=ACTION             Check authorization to perform ACTION\n"
+"  -u, --allow-user-interaction       Interact with the user if necessary\n"
+"  -d, --details=KEY VALUE            Add (KEY, VALUE) to information about the action\n"
+"  --enable-internal-agent            Use an internal authentication agent if necessary\n"
+"  --list-temp                        List temporary authorizations for current session\n"
+"  -p, --process=PID[,START_TIME]     Check authorization of specified process\n"
+"  --revoke-temp                      Revoke all temporary authorizations for current session\n"
+"  -s, --system-bus-name=BUS_NAME     Check authorization of owner of BUS_NAME\n"
+"  --version                          Show version\n"
+	     "\n"));
 }
 
 static gchar *
@@ -359,6 +365,7 @@ main (int argc, char *argv[])
 
   opt_show_help = FALSE;
   opt_show_version = FALSE;
+  g_set_prgname ("pkcheck");
   for (n = 1; n < (guint) argc; n++)
     {
       if (g_strcmp0 (argv[n], "--help") == 0)
@@ -377,7 +384,8 @@ main (int argc, char *argv[])
           n++;
           if (n >= (guint) argc)
             {
-              usage (argc, argv);
+	      g_printerr (_("%s: Argument expected after `%s'\n"),
+			  g_get_prgname (), "--process");
               goto out;
             }
 
@@ -391,7 +399,8 @@ main (int argc, char *argv[])
             }
           else
             {
-              usage (argc, argv);
+	      g_printerr (_("%s: Invalid --process value `%s'\n"),
+			  g_get_prgname (), argv[n]);
               goto out;
             }
         }
@@ -400,7 +409,8 @@ main (int argc, char *argv[])
           n++;
           if (n >= (guint) argc)
             {
-              usage (argc, argv);
+	      g_printerr (_("%s: Argument expected after `%s'\n"),
+			  g_get_prgname (), "--system-bus-name");
               goto out;
             }
 
@@ -411,7 +421,8 @@ main (int argc, char *argv[])
           n++;
           if (n >= (guint) argc)
             {
-              usage (argc, argv);
+	      g_printerr (_("%s: Argument expected after `%s'\n"),
+			  g_get_prgname (), "--action-id");
               goto out;
             }
 
@@ -425,7 +436,8 @@ main (int argc, char *argv[])
           n++;
           if (n >= (guint) argc)
             {
-              usage (argc, argv);
+	      g_printerr (_("%s: Two arguments expected after `--detail'\n"),
+			  g_get_prgname ());
               goto out;
             }
           key = argv[n];
@@ -433,7 +445,8 @@ main (int argc, char *argv[])
           n++;
           if (n >= (guint) argc)
             {
-              usage (argc, argv);
+	      g_printerr (_("%s: Two arguments expected after `--detail'\n"),
+			  g_get_prgname ());
               goto out;
             }
           value = argv[n];
@@ -464,7 +477,7 @@ main (int argc, char *argv[])
 
   if (opt_show_help)
     {
-      usage (argc, argv);
+      help ();
       ret = 0;
       goto out;
     }
@@ -487,7 +500,7 @@ main (int argc, char *argv[])
     }
   else if (subject == NULL)
     {
-      usage (argc, argv);
+      g_printerr (_("%s: Subject not specified\n"), g_get_prgname ());
       goto out;
     }
 
diff --git a/src/programs/pkttyagent.c b/src/programs/pkttyagent.c
index 488ca8b..7bd8031 100644
--- a/src/programs/pkttyagent.c
+++ b/src/programs/pkttyagent.c
@@ -24,134 +24,93 @@
 #endif
 
 #include <stdio.h>
+#include <glib/gi18n.h>
 #include <polkit/polkit.h>
 #define POLKIT_AGENT_I_KNOW_API_IS_SUBJECT_TO_CHANGE
 #include <polkitagent/polkitagent.h>
 
-static void
-usage (int argc, char *argv[])
-{
-  GError *error;
-
-  error = NULL;
-  if (!g_spawn_command_line_sync ("man pkttyagent",
-                                  NULL,
-                                  NULL,
-                                  NULL,
-                                  &error))
-    {
-      g_printerr ("Cannot show manual page: %s (%s, %d)\n",
-                  error->message, g_quark_to_string (error->domain), error->code);
-      g_error_free (error);
-    }
-}
-
-
 int
 main (int argc, char *argv[])
 {
-  gboolean opt_show_help = FALSE;
   gboolean opt_show_version = FALSE;
   gboolean opt_fallback = FALSE;
+  gchar *opt_process = NULL;
+  gchar *opt_system_bus_name = NULL;
+  gint opt_notify_fd = -1;
+  GOptionEntry options[] =
+    {
+      {
+	"fallback", 0, 0, G_OPTION_ARG_NONE, &opt_fallback,
+	N_("Don't replace existing agent if any"), NULL
+      },
+      {
+	"notify-fd", 0, 0, G_OPTION_ARG_INT, &opt_notify_fd,
+	N_("Close FD when the agent is registered"), N_("FD")
+      },
+      {
+	"process", 'p', 0, G_OPTION_ARG_STRING, &opt_process,
+	N_("Register the agent for the specified process"),
+	N_("PID[,START_TIME]")
+      },
+      {
+	"system-bus-name", 's', 0, G_OPTION_ARG_STRING, &opt_system_bus_name,
+	N_("Register the agent owner of BUS_NAME"), N_("BUS_NAME")
+      },
+      {
+	"version", 0, 0, G_OPTION_ARG_NONE, &opt_show_version,
+	N_("Show version"), NULL
+      },
+      { NULL, 0, 0, 0, NULL, NULL, NULL }
+    };
+  GOptionContext *context;
   PolkitAuthority *authority = NULL;
   PolkitSubject *subject = NULL;
   gpointer local_agent_handle = NULL;
   PolkitAgentListener *listener = NULL;
-  GVariant *options = NULL;
+  GVariant *listener_options = NULL;
   GError *error;
   GMainLoop *loop = NULL;
-  guint n;
   guint ret = 126;
-  gint notify_fd = -1;
   GVariantBuilder builder;
 
   g_type_init ();
 
-  for (n = 1; n < (guint) argc; n++)
-    {
-      if (g_strcmp0 (argv[n], "--help") == 0)
-        {
-          opt_show_help = TRUE;
-        }
-      else if (g_strcmp0 (argv[n], "--version") == 0)
-        {
-          opt_show_version = TRUE;
-        }
-      else if (g_strcmp0 (argv[n], "--fallback") == 0)
-        {
-          opt_fallback = TRUE;
-        }
-      else if (g_strcmp0 (argv[n], "--notify-fd") == 0)
-        {
-          n++;
-          if (n >= (guint) argc)
-            {
-              usage (argc, argv);
-              goto out;
-            }
-
-          if (sscanf (argv[n], "%i", &notify_fd) != 1)
-            {
-              usage (argc, argv);
-              goto out;
-            }
-        }
-      else if (g_strcmp0 (argv[n], "--process") == 0 || g_strcmp0 (argv[n], "-p") == 0)
-        {
-          gint pid;
-          guint64 pid_start_time;
-
-          n++;
-          if (n >= (guint) argc)
-            {
-              usage (argc, argv);
-              goto out;
-            }
-
-          if (sscanf (argv[n], "%i,%" G_GUINT64_FORMAT, &pid, &pid_start_time) == 2)
-            {
-              subject = polkit_unix_process_new_full (pid, pid_start_time);
-            }
-          else if (sscanf (argv[n], "%i", &pid) == 1)
-            {
-              subject = polkit_unix_process_new (pid);
-            }
-          else
-            {
-              usage (argc, argv);
-              goto out;
-            }
-        }
-      else if (g_strcmp0 (argv[n], "--system-bus-name") == 0 || g_strcmp0 (argv[n], "-s") == 0)
-        {
-          n++;
-          if (n >= (guint) argc)
-            {
-              usage (argc, argv);
-              goto out;
-            }
-
-          subject = polkit_system_bus_name_new (argv[n]);
-        }
-      else
-        {
-          break;
-        }
-    }
-
-  if (opt_show_help)
+  error = NULL;
+  context = g_option_context_new ("");
+  g_option_context_add_main_entries (context, options, GETTEXT_PACKAGE);
+  if (!g_option_context_parse (context, &argc, &argv, &error))
     {
-      usage (argc, argv);
-      ret = 0;
+      g_printerr ("%s: %s\n", g_get_prgname (), error->message);
+      g_error_free (error);
       goto out;
     }
-  else if (opt_show_version)
+
+  if (opt_show_version)
     {
       g_print ("pkttyagent version %s\n", PACKAGE_VERSION);
       ret = 0;
       goto out;
     }
 
+  if (opt_process != NULL)
+    {
+      gint pid;
+      guint64 pid_start_time;
+
+      if (sscanf (opt_process, "%i,%" G_GUINT64_FORMAT, &pid, &pid_start_time)
+	  == 2)
+	subject = polkit_unix_process_new_full (pid, pid_start_time);
+      else if (sscanf (opt_process, "%i", &pid) == 1)
+	subject = polkit_unix_process_new (pid);
+      else
+	{
+	  g_printerr (_("%s: Invalid process specifier `%s'\n"),
+		      g_get_prgname (), opt_process);
+	  goto out;
+	}
+    }
+  if (opt_system_bus_name != NULL)
+    subject = polkit_system_bus_name_new (opt_system_bus_name);
   /* Use parent process, if no subject has been specified */
   if (subject == NULL)
     {
@@ -176,7 +135,6 @@ main (int argc, char *argv[])
       g_assert (polkit_unix_process_get_start_time (POLKIT_UNIX_PROCESS (subject)) > 0);
     }
 
-  error = NULL;
   authority = polkit_authority_get_sync (NULL /* GCancellable* */, &error);
   if (authority == NULL)
     {
@@ -191,7 +149,7 @@ main (int argc, char *argv[])
     {
       g_variant_builder_init (&builder, G_VARIANT_TYPE_VARDICT);
       g_variant_builder_add (&builder, "{sv}", "fallback", g_variant_new_boolean (TRUE));
-      options = g_variant_builder_end (&builder);
+      listener_options = g_variant_builder_end (&builder);
     }
 
   error = NULL;
@@ -209,10 +167,10 @@ main (int argc, char *argv[])
                                                                     POLKIT_AGENT_REGISTER_FLAGS_RUN_IN_THREAD,
                                                                     subject,
                                                                     NULL, /* object_path */
-                                                                    options,
+                                                                    listener_options,
                                                                     NULL, /* GCancellable */
                                                                     &error);
-  options = NULL; /* consumed */
+  listener_options = NULL; /* consumed */
   g_object_unref (listener);
   if (local_agent_handle == NULL)
     {
@@ -222,11 +180,11 @@ main (int argc, char *argv[])
       goto out;
     }
 
-  if (notify_fd != -1)
+  if (opt_notify_fd != -1)
     {
-      if (close (notify_fd) != 0)
+      if (close (opt_notify_fd) != 0)
         {
-          g_printerr ("Error closing notify-fd %d: %m\n", notify_fd);
+          g_printerr ("Error closing notify-fd %d: %m\n", opt_notify_fd);
           goto out;
         }
     }
@@ -241,8 +199,8 @@ main (int argc, char *argv[])
   if (local_agent_handle != NULL)
     polkit_agent_listener_unregister (local_agent_handle);
 
-  if (options != NULL)
-    g_variant_unref (options);
+  if (listener_options != NULL)
+    g_variant_unref (listener_options);
 
   if (subject != NULL)
     g_object_unref (subject);
@@ -250,5 +208,9 @@ main (int argc, char *argv[])
   if (authority != NULL)
     g_object_unref (authority);
 
+  g_free (opt_process);
+  g_free (opt_system_bus_name);
+  g_option_context_free (context);
+
   return ret;
 }