Re: FYI, more id.c changes

[Jim Meyering]

Jim Meyering wrote:
> While investigating today's bug, I noticed that a plain old "id -G"
> would call getcon unnecessarily.  It's not going to print a context
> string, so it obviously doesn't need to call getcon.
>
> While addressing that, factoring and cleaning up, I noticed this:
>
>     Old behavior: nonsensical diagnostic, since with -Z,
>     you don't get the default format:
>
>         $ id -Z -n
>         id: cannot print only names or real IDs in default format
>
>     New: -n is ignored with --context (-Z)
>
>         $ src/id -Z -n
>         unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
>
> Considering it's at least two separate issues, I will separate this
> into two (or more) patches, with at least one more test:

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

* 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 fbbb4e0e351f528dc662064b413bcd76c44767fc 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..058622c 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))
+  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)
+  bool default_format = (just_user + just_group + just_group_list == 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 25464d7db89f873a1cb91932174033ab2a282505 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 058622c..f70f64f 100644
--- a/src/id.c
+++ b/src/id.c
@@ -181,7 +181,8 @@ main (int argc, char **argv)
     error (EXIT_FAILURE, 0,
            _("cannot print only names or real IDs in default format"));

-  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 we are on a selinux-enabled kernel, no user is specified, and
      either --context is specified or none of (-u,-g,-G) is specified,
--
1.7.10.365.g7cacb