Attachment 78941 Details for Bug 29936 – 0001-Don-t-spawn-man-for-help.patch

[patch] 0001-Don-t-spawn-man-for-help.patch

0001-Don-t-spawn-man-for-help.patch (text/plain), 17.36 KB, created by Miloslav Trmac on 2013-05-06 18:59:20 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Miloslav Trmac

Created: 2013-05-06 18:59:20 UTC

Size: 17.36 KB

patch

obsolete

>From b17cdf1f0f56dc1f1f5416888d6e9858f073e230 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= <mitr@redhat.com>
>Date: Sat, 20 Apr 2013 00:20:21 +0200
>Subject: [PATCH 1/6] 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
>---
> po/POTFILES.in            |   3 +
> src/programs/pkaction.c   |  91 +++++++++---------------
> src/programs/pkcheck.c    |  55 ++++++++------
> src/programs/pkttyagent.c | 178 ++++++++++++++++++----------------------------
> 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 2d8c90d..d779b3c 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,19 +88,35 @@ 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;
>   PolkitActionDescription *description;
>   GError *error;
> 
>-  action_id = NULL;
>+  opt_action_id = NULL;
>+  context = NULL;
>   authority = NULL;
>   actions = NULL;
>   description = NULL;
>@@ -124,50 +124,25 @@ main (int argc, char *argv[])
> 
>   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)
>     {
>@@ -187,7 +162,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)
>         {
>@@ -196,7 +171,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;
>@@ -205,7 +180,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;
>         }
>     }
>@@ -229,11 +204,13 @@ main (int argc, char *argv[])
>   if (description != NULL)
>     g_object_unref (description);
> 
>-  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;
> }
>-- 
>1.8.1.4
>

Actions: View | Splinter Review

Attachments on bug 29936: 78258 | 78259 | 78260 | 78261 | 78262 | 78263 | 78941 | 78942 | 105344 | 132120

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.