Re: FYI, more id.c changes

[Jim Meyering]

Jim Meyering wrote:
>> Just as well that I separated them.
>> In doing so, I found an additional "argc - optind" to factor out.
>> I haven't added a test for the second or third patches -- out of time,
>> and they don't seem worth it.  However, if someone else would like to,
>> you're welcome.
>>
>>>From a6719d9f7252bbd85eaab840a61dfc93d57b05c5 Mon Sep 17 00:00:00 2001
>> From: Jim Meyering <address@hidden>
>> Date: Fri, 27 Apr 2012 20:44:58 +0200
>> Subject: [PATCH 1/3] maint: id: minor factorization

Here's the respin:
1/3 is the same, but the other two now actually compile.

>From a6719d9f7252bbd85eaab840a61dfc93d57b05c5 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 27 Apr 2012 20:44:58 +0200
Subject: [PATCH 1/3] maint: id: minor factorization

* src/id.c (main): Factor out uses of "argc - optind".
Move option-consistency checks to precede the potential getcon call.
---
 src/id.c |   23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/id.c b/src/id.c
index c600e63..e1b51e7 100644
--- a/src/id.c
+++ b/src/id.c
@@ -163,34 +163,35 @@ main (int argc, char **argv)
         }
     }

-  if (1 < argc - optind)
+  size_t n_ids = argc - optind;
+  if (1 < n_ids)
     {
       error (0, 0, _("extra operand %s"), quote (argv[optind + 1]));
       usage (EXIT_FAILURE);
     }

-  if (argc - optind == 1 && just_context)
+  if (n_ids && just_context)
     error (EXIT_FAILURE, 0,
            _("cannot print security context when user specified"));

+  if (just_user + just_group + just_group_list + just_context > 1)
+    error (EXIT_FAILURE, 0, _("cannot print \"only\" of more than one 
choice"));
+
+  if (just_user + just_group + just_group_list == 0 && (use_real || use_name))
+    error (EXIT_FAILURE, 0,
+           _("cannot print only names or real IDs in default format"));
+
   /* If we are on a selinux-enabled kernel and no user is specified,
      get our context. Otherwise, leave the context variable alone -
      it has been initialized known invalid value and will be not
      displayed in print_full_info() */
-  if (selinux_enabled && argc == optind)
+  if (selinux_enabled && n_ids == 0)
     {
       if (getcon (&context) && just_context)
         error (EXIT_FAILURE, 0, _("can't get process context"));
     }

-  if (just_user + just_group + just_group_list + just_context > 1)
-    error (EXIT_FAILURE, 0, _("cannot print \"only\" of more than one 
choice"));
-
-  if (just_user + just_group + just_group_list == 0 && (use_real || use_name))
-    error (EXIT_FAILURE, 0,
-           _("cannot print only names or real IDs in default format"));
-
-  if (argc - optind == 1)
+  if (n_ids == 1)
     {
       struct passwd *pwd = getpwnam (argv[optind]);
       if (pwd == NULL)
--
1.7.10.365.g7cacb


>From c8b8279d2bd9ed1005ebbcca018446d3a245b97c Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 27 Apr 2012 21:24:03 +0200
Subject: [PATCH 2/3] id: don't call getcon unnecessarily

* src/id.c (main): Invocations like "id" and "id -G" would call getcon
to determine the current security context even though that result would
not be used.  Similarly, when POSIXLY_CORRECT is set.  Rearrange
conditionals and hoist the POSIXLY_CORRECT test so that we call
getcon only when necessary.
---
 src/id.c |   21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/id.c b/src/id.c
index e1b51e7..da81422 100644
--- a/src/id.c
+++ b/src/id.c
@@ -177,16 +177,23 @@ main (int argc, char **argv)
   if (just_user + just_group + just_group_list + just_context > 1)
     error (EXIT_FAILURE, 0, _("cannot print \"only\" of more than one 
choice"));

-  if (just_user + just_group + just_group_list == 0 && (use_real || use_name))
+  bool default_format = (just_user + just_group + just_group_list == 0);
+
+  if (default_format && (use_real || use_name))
     error (EXIT_FAILURE, 0,
            _("cannot print only names or real IDs in default format"));

-  /* If we are on a selinux-enabled kernel and no user is specified,
-     get our context. Otherwise, leave the context variable alone -
-     it has been initialized known invalid value and will be not
-     displayed in print_full_info() */
-  if (selinux_enabled && n_ids == 0)
+  /* If we are on a selinux-enabled kernel, no user is specified, and
+     either --context is specified or none of (-u,-g,-G) is specified,
+     and we're not in POSIXLY_CORRECT mode, get our context.  Otherwise,
+     leave the context variable alone - it has been initialized to an
+     invalid value that will be not displayed in print_full_info().  */
+  if (selinux_enabled
+      && n_ids == 0
+      && (just_context ||
+          (default_format && ! getenv ("POSIXLY_CORRECT"))))
     {
+      /* Report failure only if --context (-Z) was explicitly requested.  */
       if (getcon (&context) && just_context)
         error (EXIT_FAILURE, 0, _("can't get process context"));
     }
@@ -361,6 +368,6 @@ print_full_info (const char *username)

   /* POSIX mandates the precise output format, and that it not include
      any context=... part, so skip that if POSIXLY_CORRECT is set.  */
-  if (context != NULL && ! getenv ("POSIXLY_CORRECT"))
+  if (context)
     printf (_(" context=%s"), context);
 }
--
1.7.10.365.g7cacb


>From c83f723267dff03e954ba77454d51a691dfba1c0 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 27 Apr 2012 21:30:52 +0200
Subject: [PATCH 3/3] id: -Zn/-Zr: avoid an invalid diagnostic

* src/id.c (main): Using -Z with -r or -n would fail with "id: cannot
print only names or real IDs in default format", in spite of that "-Z",
which specifies a non-default format.  Now, it succeeds and ignores
the -n or -r option.  The error was that the test for default_format
was not updated when I added the new --context (-Z) option in
commit v6.9-33-g5320d0f.
---
 src/id.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/id.c b/src/id.c
index da81422..158715e 100644
--- a/src/id.c
+++ b/src/id.c
@@ -177,7 +177,8 @@ main (int argc, char **argv)
   if (just_user + just_group + just_group_list + just_context > 1)
     error (EXIT_FAILURE, 0, _("cannot print \"only\" of more than one 
choice"));

-  bool default_format = (just_user + just_group + just_group_list == 0);
+  bool default_format = (just_user + just_group + just_group_list
+                         + just_context == 0);

   if (default_format && (use_real || use_name))
     error (EXIT_FAILURE, 0,
--
1.7.10.365.g7cacb