[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Found and fixed bug in groups(1). Want the diff?
From: |
Jim Meyering |
Subject: |
Re: Found and fixed bug in groups(1). Want the diff? |
Date: |
Tue, 26 Sep 2006 11:46:30 +0200 |
Iain Calder <address@hidden> wrote:
> OS: GNU/Linux Debian 3.1 (sarge)
> Command: /usr/bin/groups
> Problem: Doesn't return failure when unable to display output.
> Notes: groups(1) is a small shell script; 42 lines excluding
> the copyright comments.
>
> The author went to great lengths to return failure when the
> program is unable to display output, but didn't do so on the main
> code path. Thus the program's behaviour is inconsistent. Witness:
>
> $ groups --version; echo $?
> groups (GNU coreutils) 5.2.1
> 0
> $ groups --version >&-; echo $?
> 1 <--- echo failure reflected in exit status
>
> $ groups ic; echo $?
> ic : ic dialout cdrom floppy audio src video plugdev
> 0
> $ groups ic >&-; echo $?
> 0 <--- echo failed, yet status is successful!
>
> The code was obfuscated. I fixed the bug, made the script
> shorter as well as cleaner, then added a couple of comments.
> Is this code being maintained? To whom should I send my diffs?
With no diffs, I can only guess, but here's a small additional change:
2006-09-26 Jim Meyering <address@hidden>
* src/groups.sh: When invoked with 0 or 1 argument, just exec "id".
Rewrite to avoid using temporary, $status.
Index: src/groups.sh
===================================================================
RCS file: /fetish/cu/src/groups.sh,v
retrieving revision 1.20
diff -u -r1.20 groups.sh
--- src/groups.sh 26 Sep 2006 09:28:18 -0000 1.20
+++ src/groups.sh 26 Sep 2006 09:42:06 -0000
@@ -53,18 +53,17 @@
* ) ;;
esac
-if [ $# -eq 0 ]; then
- id -Gn
- fail=$?
-else
- for name in "$@"; do
- groups=`id -Gn -- $name`
- status=$?
- if test $status = 0; then
- echo $name : $groups || fail=1
- else
- fail=$status
- fi
- done
-fi
+# With fewer than two arguments, simply exec "id".
+case $# in
+ 0|1) exec id -Gn "$@" ;;
+esac
+
+# With more, we need a loop, and be sure to exit nonzero upon failure.
+for name in "$@"; do
+ if groups=`id -Gn -- $name`; then
+ echo $name : $groups || fail=1
+ else
+ fail=1
+ fi
+done
exit $fail