Jim Meyering wrote: > 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 <[email protected]> > 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 <[email protected]> > 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);
Well, don't waste time looking at those. In a last-minute rebase, I didn't notice that while resolving a conflict I moved the declaration of default_format to be after the first use. Oops. Adjusting...
