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); + + /* 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 <[email protected]> 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
