Re: [PATCH] 'id' prints incorrectly groups for the session

[Bernhard Voelker]

On 06/26/2014 03:53 AM, Pádraig Brady wrote:
> diff --git a/tests/id/setgid.sh b/tests/id/setgid.sh
> index aa43ea3..a81b42c 100755
> --- a/tests/id/setgid.sh
> +++ b/tests/id/setgid.sh
> @@ -1,5 +1,5 @@
>  #!/bin/sh
> -# Verify that id -G prints the right group when run set-GID.
> +# Verify that id [-G] prints the right group when run set-GID.
>  
>  # Copyright (C) 2012-2014 Free Software Foundation, Inc.
>  
> @@ -27,9 +27,14 @@ gp1=$(expr $g + 1)
>  
>  echo $gp1 > exp || framework_failure_
>  
> +# With coreutils-8.16 and earlier, id -G would print both: $gp1 $g
>  chroot --user=$NON_ROOT_USERNAME:$gp1 --groups='' / env PATH="$PATH" \

shouldn't we better avoid group name resolution here?

- chroot --user=$NON_ROOT_USERNAME:$gp1 --groups='' / env PATH="$PATH" \
+ chroot --user=$NON_ROOT_USERNAME:+$gp1 --groups='' / env PATH="$PATH" \

>    id -G > out || fail=1
>  compare exp out || fail=1
> -# With coreutils-8.16 and earlier, id -G would print both: $gp1 $g
> +
> +# With coreutils-8.22 and earlier, id would erroneously print groups=$g
> +chroot --user=$NON_ROOT_USERNAME:$gp1 --groups='' / env PATH="$PATH" \

Likewise.

> +  id > out || fail=1
> +grep -F "groups=$gp1" out || fail=1
>  
>  Exit $fail

Another minor nit:
for a better diagnostic, it'd be better to use the construct
we already use in many places:

  ... || { cat out; fail=1; }

Otherwise +1.

Thanks & have a nice day,
Berny