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