Subject: Re: bug#9987: [PATCH] groups,id: add -0, --null option

From: Bernhard Voelker <[email-masked]>
Date: Tue, 17 Sep 2013 00:21:35 +0200

On 09/16/2013 07:40 AM, Bernhard Voelker wrote:
> On 09/16/2013 02:18 AM, Pádraig Brady wrote:
>> Yes please use -z, --zero for most consistency.
> 
> Ok, fine, thanks.

Here comes the new patch with {groups,id} -z,--zero.

Have a nice day,
Berny
>From b01d789b63e0c7721f308d4c292f17237f342d99 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <[email-masked]>
Date: Tue, 17 Sep 2013 00:03:30 +0200
Subject: [PATCH] groups,id: add -z, --zero option

* src/group-list.h (print_group_list): Add a parameter for the
separator of type char.
* src/group-list.c (print_group_list): Likewise, and use it instead
of a white space character to separate the group entries.
* src/groups.c (longopts): Add an array element for the new long option.
(usage): Document the new options.
(main): Define the bool flag opt_zero indicating the use of the new
options.  In the getopt_long loop, handle it.
In the case without a given USERNAME argument, pass either a white space
or a NUL character to print_group_list() - depending on the above flag.
Only output the final newline character unless the --zero option has
been specified.
Likewise in the case with a given USERNAME and when the --zero option
has been specified, plus output 2 NULs separating consecutive users.
* src/id.c (longopts):  Add array element for the new long option.
(usage): Document the new options.  While at it, fix the alignment
of the descriptions to match that of HELP_OPTION_DESCRIPTION.
(main): Define the bool flag opt_zero indicating the use of the
new options.  In the getopt_long loop, handle it.
Output an error diagnostic in the case the --zero option has been
specified together with the default format.
In the case of -gG, pass either a NUL or a white space character to
print_group_list() - depending on the above new flag.
Only output the final newline character unless the --zero option has
been specified.
* doc/coreutils.texi (optZeroEntry):  Add new macro describing the
new options.
(id invocation): Use the above new macro and mention that the new
options are not permitted when using the default format.
Move the @exitstatus macro down after @primaryAndSupplementaryGroups
in order to be consistent with other info pages.
(groups invocation): Instead of saying that the --help and --version
options are the only ones, add a @ref to these common options and
use the above new macro to explain the new options.
Document the resulting output syntax for the cases without, with one
and with more user names specified.
Move @exitstatus down after @primaryAndSupplementaryGroups here, too.
* tests/misc/groups-id-zero.sh: Add new test exercising the new options.
* tests/local.mk (all_tests): Reference it.
* NEWS (New features): Mention the new options.
Fixes http://bugs.gnu.org/9987
---
 NEWS                         |   3 +
 doc/coreutils.texi           |  58 ++++++++++++++++++--
 src/group-list.c             |   6 +-
 src/group-list.h             |   2 +-
 src/groups.c                 |  41 +++++++++++---
 src/id.c                     |  37 +++++++++----
 tests/local.mk               |   1 +
 tests/misc/groups-id-zero.sh | 128 +++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 248 insertions(+), 28 deletions(-)
 create mode 100755 tests/misc/groups-id-zero.sh

diff --git a/NEWS b/NEWS
index d26722d..e8259eb 100644
--- a/NEWS
+++ b/NEWS
@@ -44,6 +44,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   du accepts a new option: --inodes to show the number of inodes instead
   of the blocks used.
 
+  groups and id accept a new option: --zero (-z) to separate the output
+  entries by a NUL instead of a white space character.
+
   id and ls with -Z report the SMACK security context where available.
   mkdir, mkfifo and mknod with -Z set the SMACK context where available.
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 21216b4..2715a54 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -14473,7 +14473,28 @@ followed by the corresponding user or group name in parentheses.
 The options cause @command{id} to print only part of the above information.
 Also see @ref{Common options}.
 
+@macro optZeroEntry{cmd}
+@item -z
+@opindex -z
+@itemx --zero
+@opindex --zero
+@cindex output NUL-byte-terminated entries
+Output a zero byte (ASCII NUL) to separate the entries, rather than a whitespace
+character.  Also elide the newline character at the end of the output.
+This option enables other programs to parse the output of @command{\cmd\}
+even when that output would contain data with embedded whitespaces.
+@end macro
+
 @table @samp
+@optZeroEntry{id}
+This option is not permitted when using the default format.
+
+Example:
+@example
+$ id -Gn --zero
+users <NUL> devs
+@end example
+
 @item -g
 @itemx --group
 @opindex -g
@@ -14518,8 +14539,6 @@ set the exit status to 1.
 
 @end table
 
-@exitstatus
-
 @macro primaryAndSupplementaryGroups{cmd,arg}
 Primary and supplementary groups for a process are normally inherited
 from its parent and are usually unchanged since login.  This means
@@ -14530,6 +14549,8 @@ database to be consulted afresh, and so will give a different result.
 @end macro
 @primaryAndSupplementaryGroups{id,user argument}
 
+@exitstatus
+
 @node logname invocation
 @section @command{logname}: Print current login name
 
@@ -14587,13 +14608,38 @@ groups [@var{username}]@dots{}
 
 The group lists are equivalent to the output of the command @samp{id -Gn}.
 
-@primaryAndSupplementaryGroups{groups,list of users}
+The program accepts the following options.  Also see @ref{Common options}.
 
-The only options are @option{--help} and @option{--version}.  @xref{Common
-options}.
+@table @samp
+@optZeroEntry{id}
 
-@exitstatus
+Example:
+@example
+$ groups -z
+users <NUL> devs
+@end example
+
+When a @var{username} is specified, @command{groups} outputs a NUL byte after
+the @var{username} instead of the colon (@samp{" : "}).
+
+@example
+$ groups --zero matt
+matt <NUL> users
+@end example
+
+When further @var{username}s are specified, then @command{groups} outputs
+2 NUL bytes before continuing with the next user:
 
+@example
+$ groups john matt -z
+john <NUL> users <NUL> devs <NUL> <NUL>
+matt <NUL> users
+@end example
+@end table
+
+@primaryAndSupplementaryGroups{groups,list of users}
+
+@exitstatus
 
 @node users invocation
 @section @command{users}: Print login names of users currently logged in
diff --git a/src/group-list.c b/src/group-list.c
index 7d4995b..a76ee00 100644
--- a/src/group-list.c
+++ b/src/group-list.c
@@ -35,7 +35,7 @@
 extern bool
 print_group_list (const char *username,
                   uid_t ruid, gid_t rgid, gid_t egid,
-                  bool use_names)
+                  bool use_names, char separator)
 {
   bool ok = true;
   struct passwd *pwd = NULL;
@@ -52,7 +52,7 @@ print_group_list (const char *username,
 
   if (egid != rgid)
     {
-      putchar (' ');
+      putchar (separator);
       if (!print_group (egid, use_names))
         ok = false;
     }
@@ -79,7 +79,7 @@ print_group_list (const char *username,
     for (i = 0; i < n_groups; i++)
       if (groups[i] != rgid && groups[i] != egid)
         {
-          putchar (' ');
+          putchar (separator);
           if (!print_group (groups[i], use_names))
             ok = false;
         }
diff --git a/src/group-list.h b/src/group-list.h
index 3fac887..573de1d 100644
--- a/src/group-list.h
+++ b/src/group-list.h
@@ -16,4 +16,4 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 bool print_group (gid_t, bool);
-bool print_group_list (const char *, uid_t, gid_t, gid_t, bool);
+bool print_group_list (const char *, uid_t, gid_t, gid_t, bool, char);
diff --git a/src/groups.c b/src/groups.c
index 53332d5..653861e 100644
--- a/src/groups.c
+++ b/src/groups.c
@@ -38,6 +38,7 @@
 
 static struct option const longopts[] =
 {
+  {"zero", no_argument, NULL, 'z'},
   {GETOPT_HELP_OPTION_DECL},
   {GETOPT_VERSION_OPTION_DECL},
   {NULL, 0, NULL, 0}
@@ -56,6 +57,9 @@ Print group memberships for each USERNAME or, if no USERNAME is specified, for\
 \n\
 the current process (which may differ if the groups database has changed).\n"),
              stdout);
+      fputs (_("\
+  -z, --zero     separate entries by a NUL, not whitespace\n\
+"), stdout);
       fputs (HELP_OPTION_DESCRIPTION, stdout);
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
       emit_ancillary_info ();
@@ -68,9 +72,11 @@ main (int argc, char **argv)
 {
   int optc;
   bool ok = true;
+  bool opt_zero = false;
   gid_t rgid, egid;
   uid_t ruid;
 
+
   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
   setlocale (LC_ALL, "");
@@ -82,10 +88,14 @@ main (int argc, char **argv)
   /* Processing the arguments this way makes groups.c behave differently to
    * groups.sh if one of the arguments is "--".
    */
-  while ((optc = getopt_long (argc, argv, "", longopts, NULL)) != -1)
+  while ((optc = getopt_long (argc, argv, "z", longopts, NULL)) != -1)
     {
       switch (optc)
         {
+        case 'z':
+          opt_zero = true;
+          break;
+
         case_GETOPT_HELP_CHAR;
         case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
         default:
@@ -114,13 +124,17 @@ main (int argc, char **argv)
       if (rgid == NO_GID && errno)
         error (EXIT_FAILURE, errno, _("cannot get real GID"));
 
-      if (!print_group_list (NULL, ruid, rgid, egid, true))
+      if (!print_group_list (NULL, ruid, rgid, egid, true,
+                             opt_zero ? '\0' : ' '))
         ok = false;
-      putchar ('\n');
+
+      if (!opt_zero)
+        putchar ('\n');
     }
   else
     {
-      /* At least one argument.  Divulge the details of the specified users. */
+      /* At least one argument.  Divulge the details of the specified users.  */
+      int nusers = 0;
       while (optind < argc)
         {
           struct passwd *pwd = getpwnam (argv[optind]);
@@ -129,10 +143,23 @@ main (int argc, char **argv)
           ruid = pwd->pw_uid;
           rgid = egid = pwd->pw_gid;
 
-          printf ("%s : ", argv[optind]);
-          if (!print_group_list (argv[optind++], ruid, rgid, egid, true))
+          if (opt_zero)
+            {
+              /* Separate consecutive user output by two NULs.  */
+              if (nusers++)
+                printf ("%c%c", '\0', '\0');
+              printf ("%s%c", argv[optind], '\0');
+            }
+          else
+            {
+              printf ("%s : ", argv[optind]);
+            }
+          if (!print_group_list (argv[optind++], ruid, rgid, egid, true,
+                                 opt_zero ? '\0' : ' '))
             ok = false;
-          putchar ('\n');
+
+          if (!opt_zero)
+            putchar ('\n');
         }
     }
 
diff --git a/src/id.c b/src/id.c
index 3e7016f..7aa50e1 100644
--- a/src/id.c
+++ b/src/id.c
@@ -61,6 +61,7 @@ static security_context_t context = NULL;
 
 static struct option const longopts[] =
 {
+  {"zero", no_argument, NULL, 'z'},
   {"context", no_argument, NULL, 'Z'},
   {"group", no_argument, NULL, 'g'},
   {"groups", no_argument, NULL, 'G'},
@@ -83,14 +84,18 @@ usage (int status)
       fputs (_("\
 Print user and group information for the specified USERNAME,\n\
 or (when USERNAME omitted) for the current user.\n\
-\n\
-  -a              ignore, for compatibility with other versions\n\
-  -Z, --context   print only the security context of the current user\n\
-  -g, --group     print only the effective group ID\n\
-  -G, --groups    print all group IDs\n\
-  -n, --name      print a name instead of a number, for -ugG\n\
-  -r, --real      print the real ID instead of the effective ID, with -ugG\n\
-  -u, --user      print only the effective user ID\n\
+\n"),
+             stdout);
+      fputs (_("\
+  -a             ignore, for compatibility with other versions\n\
+  -Z, --context  print only the security context of the current user\n\
+  -g, --group    print only the effective group ID\n\
+  -G, --groups   print all group IDs\n\
+  -n, --name     print a name instead of a number, for -ugG\n\
+  -r, --real     print the real ID instead of the effective ID, with -ugG\n\
+  -u, --user     print only the effective user ID\n\
+  -z, --zero     separate entries by a NUL, not whitespace;\n\
+                   not permitted in default format\n\
 "), stdout);
       fputs (HELP_OPTION_DESCRIPTION, stdout);
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
@@ -109,6 +114,7 @@ main (int argc, char **argv)
   int optc;
   int selinux_enabled = (is_selinux_enabled () > 0);
   bool smack_enabled = is_smack_enabled ();
+  bool opt_zero = false;
 
   /* If true, output the list of all group IDs. -G */
   bool just_group_list = false;
@@ -127,7 +133,7 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  while ((optc = getopt_long (argc, argv, "agnruGZ", longopts, NULL)) != -1)
+  while ((optc = getopt_long (argc, argv, "agnruzGZ", longopts, NULL)) != -1)
     {
       switch (optc)
         {
@@ -162,6 +168,9 @@ main (int argc, char **argv)
         case 'u':
           just_user = true;
           break;
+        case 'z':
+          opt_zero = true;
+          break;
         case 'G':
           just_group_list = true;
           break;
@@ -193,6 +202,10 @@ main (int argc, char **argv)
     error (EXIT_FAILURE, 0,
            _("cannot print only names or real IDs in default format"));
 
+  if (default_format && opt_zero)
+    error (EXIT_FAILURE, 0,
+           _("option --zero not permitted in default format"));
+
   /* If we are on a SELinux/SMACK-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,
@@ -269,7 +282,8 @@ main (int argc, char **argv)
     }
   else if (just_group_list)
     {
-      if (!print_group_list (argv[optind], ruid, rgid, egid, use_name))
+      if (!print_group_list (argv[optind], ruid, rgid, egid, use_name,
+                             opt_zero ? '\0' : ' '))
         ok = false;
     }
   else if (just_context)
@@ -280,7 +294,8 @@ main (int argc, char **argv)
     {
       print_full_info (argv[optind]);
     }
-  putchar ('\n');
+  if (default_format || !opt_zero)
+    putchar ('\n');
 
   exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
 }
diff --git a/tests/local.mk b/tests/local.mk
index b00ff59..42cca9b 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -271,6 +271,7 @@ all_tests =					\
   tests/misc/false-status.sh			\
   tests/misc/fold.pl				\
   tests/misc/groups-dash.sh			\
+  tests/misc/groups-id-zero.sh			\
   tests/misc/groups-version.sh			\
   tests/misc/head-c.sh				\
   tests/misc/head-pos.sh			\
diff --git a/tests/misc/groups-id-zero.sh b/tests/misc/groups-id-zero.sh
new file mode 100755
index 0000000..0f9a101
--- /dev/null
+++ b/tests/misc/groups-id-zero.sh
@@ -0,0 +1,128 @@
+#!/bin/sh
+# Exercise "groups --zero" and "id --zero".
+
+# Copyright (C) 2013 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ groups id
+require_perl_
+
+#
+# Converting back null-separated output of id(1) / groups(1) without a USERNAME.
+# Format:    group1 <NUL> group2 <NUL> group3
+# Result:    group1 <SP> group2 <SP> group3 <NL>
+# Just replace all NULs by whitespace (SP) and terminate with a newline (NL).
+#
+cat <<\EOF > conv-basic || framework_failure_
+$_ = <>;
+s/\0/ /g;
+print $_ . "\n";
+EOF
+
+#
+# Converting back null-separated output of groups(1) with USERNAME(s).
+# Format:
+#   user1 <NUL> group1 <NUL> <NUL> user2 <NUL> group2 <NUL> group3 <EOF>
+# Result:
+#   user1 : group1 <NL> user2 : group2 <SP> group3 <NL> <EOF>
+# Split up records by double-NUL, then
+# split up the line by a single NUL into USERNAME and the list of group IDs,
+# and then print each group - separated by a whitespace;
+# Terminate a record with a newline.
+#
+cat <<\EOF > conv-w-user || framework_failure_
+foreach (split ("\0\0", <>, -1))
+  {
+    my ($user, @groups) = split ("\0");
+    print $user . " :";
+    for (@groups)
+      {
+        print " " . $_;
+      }
+    print "\n";
+  }
+EOF
+
+u="$( id -nu )"
+groups "$u" > exp || fail=1
+
+# For the bulk test below, verify that xargs works and create a list of users.
+echo "$u" | xargs groups >out 2>err \
+  && compare exp out \
+  && compare /dev/null err \
+  && getent passwd | cut -d: -f1 > users \
+  && [ $(wc -l < users) -gt 0 ] \
+  && runbulk=1 \
+  || runbulk=0
+
+# Exercise "group -z", i.e. without a USERNAME.
+groups > exp || fail=1
+groups --zero > out || fail=1
+$PERL conv-basic out > out2 || framework_failure_
+compare exp out2 || fail=1
+
+# Exercise "group -z user1".
+groups "$u" > exp || fail=1
+groups -z "$u" > out || fail=1
+$PERL conv-w-user out > out2 || framework_failure_
+compare exp out2 || fail=1
+
+if [ "$u" != "root" ] && groups "root" >/dev/null
+then
+  # Exercise another user name ...
+  groups "root" > exp || fail=1
+  groups --zero "root" > out || fail=1
+  $PERL conv-w-user out > out2 || framework_failure_
+  compare exp out2 || fail=1
+
+  # ... and both together.
+  groups "$u" "root" > exp || fail=1
+  groups --zero "$u" "root" > out || fail=1
+  $PERL conv-w-user out > out2 || framework_failure_
+  compare exp out2 || fail=1
+else
+  # Fall back to using the same user name twice.
+  groups "$u" "$u" > exp || fail=1
+  groups --zero "$u" "$u" > out || fail=1
+  $PERL conv-w-user out > out2 || framework_failure_
+  compare exp out2 || fail=1
+fi
+
+# Exercise bulk "group -z user1 user2 ...", if possible.
+if [ $runbulk = 1 ] ; then
+  xargs groups <users >exp || fail=1
+  xargs groups -z <users >out || fail=1
+  $PERL conv-w-user out > out2 || framework_failure_
+  compare exp out2 || fail=1
+fi
+
+# id(1) should refuse --zero in default format.
+id --zero > exp 2>err && fail=1
+grep 'option --zero not permitted in default format' err || fail=1
+
+# Exercise "id -z" with various options.
+for o in g gr G Gr u ur ; do
+  for n in "" n ; do
+    for a in "" $u root ; do
+      id -${o}${n}  $a > exp || fail=1
+      id -${o}${n}z $a > out || fail=1
+      $PERL conv-basic out > out2 || framework_failure_
+      compare exp out2 || fail=1
+    done
+  done
+done
+
+Exit $fail
-- 
1.8.3.1