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

[Pádraig Brady]

On 06/26/2014 06:44 AM, Bernhard Voelker wrote:
> 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; }

Cool, that saves 2 syscalls per id invocation.
We can save another 2 by also using + for the user.
Pushed with these changes.

thanks!
Pádraig.

diff --git a/tests/id/setgid.sh b/tests/id/setgid.sh
index a81b42c..e0543f4 100755
--- a/tests/id/setgid.sh
+++ b/tests/id/setgid.sh
@@ -20,7 +20,8 @@
 print_ver_ id
 require_root_

-g=$(id -u $NON_ROOT_USERNAME) || framework_failure_
+u=$(id -u $NON_ROOT_USERNAME) || framework_failure_
+g=u

 # Construct a different group number.
 gp1=$(expr $g + 1)
@@ -28,13 +29,13 @@ 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" \
+chroot --user=+$u:+$gp1 --groups='' / env PATH="$PATH" \
   id -G > out || fail=1
-compare exp out || fail=1
+compare exp out || { cat out; fail=1; }

 # With coreutils-8.22 and earlier, id would erroneously print groups=$g
-chroot --user=$NON_ROOT_USERNAME:$gp1 --groups='' / env PATH="$PATH" \
+chroot --user=+$u:+$gp1 --groups='' / env PATH="$PATH" \
   id > out || fail=1
-grep -F "groups=$gp1" out || fail=1
+grep -F "groups=$gp1" out || { cat out; fail=1; }

 Exit $fail