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

[Pádraig Brady]

On 09/19/2013 12:24 AM, Bernhard Voelker wrote:
> On 09/18/2013 09:56 PM, Pádraig Brady wrote:
>> > On 09/18/2013 07:46 PM, Bernhard Voelker wrote:
>>> >> a) add the -z option only to id.c leaving groups(1) untouched.
>> > I'd vote for a) actually.
> Great, that makes the change much smaller. ;-)
> 
> The patch is attached.

> diff --git a/NEWS b/NEWS
> index d26722d..5891def 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.
>  
> +  id accepts a new option: --zero (-z) to separate the output entries by
> +  a NUL instead of a white space character.

s/separate/delimit/

>    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..327254b 100644
> --- a/doc/coreutils.texi
> +++ b/doc/coreutils.texi
> @@ -14516,9 +14516,20 @@ Print only the security context of the current user.
>  If SELinux is disabled then print a warning and
>  set the exit status to 1.
>  
> address@hidden table
> address@hidden -z
> address@hidden --zero
> address@hidden -z
> address@hidden --zero
> +Separate output items with NUL characters.

s/Separate/Delimit/

> +This option is not permitted when using the default format.
>  
> address@hidden
> +Example:
> address@hidden
> +$ id -Gn --zero
> +users <NUL> devs

users <NUL> devs <NUL>

> diff --git a/tests/misc/id-zero.sh b/tests/misc/id-zero.sh

> +id root || fail=1

Is 'root' a guaranteed name?
misc/chroot-credentials.sh since v7.4-16-gc45c51f assumes so.
I'm still not convinced (and will look at addressing that separately).
Do we even need to treat root specially. Can we just drop explicit root entry?

> +# Create a nice list of users (ignoring errors) ...
> +getent passwd | head -n 100 | cut -d: -f1 > users

test -s users || skip_ 'Failed to read the passwd database with getent'
Also why so many (100). The test is a bit slow and you would get
the same coverage from a couple of entries?

> +# ... and ensure it contains at least 'root', "$u", and an
> +# empty line for the tests without a user argument below.
> +printf "%s\n" root "$u" '' >> users || framework_failure_
> +
> +# id(1) should refuse --zero in default format.
> +id --zero > out 2>err && fail=1
> +compare /dev/null out || fail=1

> +grep 'option --zero not permitted in default format' err || fail=1

Better to:
echo 'option ...' > exp-err
compare exp-err err || fail=1

> +
> +# Exercise "id -z" with various options.
> +printf "\n" > exp || framework_failure_

Minor point, but it's better to use '' quotes when not interpolating,
here and throughout the test.

> +cp /dev/null  out || framework_failure_

:> out

thanks!
Pádraig.