>Number:         10259
>Category:       misc
>Synopsis:       example in getopt(1) man page is broken
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    misc-bug-people
>State:          open
>Class:          doc-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jun 01 19:56:00 PDT 2000
>Closed-Date:
>Last-Modified:
>Originator:     Jim Bernard
>Release:        May 30, 2000
>Organization:
	Speaking for myself
>Environment:
System: NetBSD zoo 1.4Y NetBSD 1.4Y (ZOO-$Revision: 1.52 $) #0: Fri May 26 14:58:14 MDT 2000 jim@zoo:/home/tmp/compile/sys/arch/i386/compile/ZOO i386


>Description:
	The usage example (under the heading EXAMPLE) in the getopt(1) man
	page is incorrect.  While it is true that when executed as shown
	in the example, a seemingly correct result is obtained, that is just
	an accident specific to the assumed input.  For example, if one
	executes the example with the command line "cmd -a -c arg" (no
	filename arguments), the shell complains: "shift: can't shift that
	many".  A slightly extended version of the script (which supports
	two options that take arguments), presented below, exhibits more
	catastrophic failure, yielding incorrect values for variables it
	sets.  A working version of the code is also given in the script
	below.

	The source of this problem is that the example performs an extra
	shift inside the case statement when an option that takes an
	argument is processed.  However, the shift before the case
	statement is processed on every traversal of the loop, which includes
	the arguments of the options that take arguments.  In those cases,
	the extra shift done in the case statement is superfluous, causing
	the processing to get out of sync.  If there are enough extra
	arguments at the end of the command line and only one extra shift
	from an option argument, there will be no complaints, and the example
	will appear to work, even though the command line is not processed
	correctly and one of the trailing "filename" arguments is lost.  In
	other circumstances (see below), the incorrect processing can cause
	more obvious failures.  (I imagine the example was originally
	constructed under the assumption that the shifts caused the list
	being processed by the for statement to be modified on each
	iteration, but in fact it is static.)

	In addition, giving an argument of "-a" to the option "-b" will cause
	the code in the example to believe erroneously that the "-a" flag was
	present (see discussion below in the fix section).

>How-To-Repeat:
	Try this script, with, e.g., "cmd -a -b bb -c cc" with both versions
	of the code; note the incorrect processing of the command line
	with the first form (which corresponds to the approach used in the
	example in the man page) and the correct processing with the second
	form.

#! /bin/sh

args=`getopt ab:c: $*`
if [ $? != 0 ]
then
	echo 'Usage: ...'
	exit 2
fi
set -- $args

# This doesn't work [it's like the example in getopt(1)]:
for i; do
	echo "arg: $i"
	echo "  arg 1 before shift: $1"
	shift
	echo "  arg 1 after shift: $1"
	case "$i" in
		-a)	flag=$i		;;
		-b)	barg=$1;	shift;;
		-c)	carg=$1;	shift;;
		--)			break;;
	esac
done

# This does work:
#while [ $# > 0 ]; do
#	echo "arg 1 before case: $1"
#	case "$1" in
#		-a)	flag=$1		;;
#		-b)	barg=$2;	shift;;
#		-c)	carg=$2;	shift;;
#		--)			shift; break;;
#	esac
#	echo "  arg 1 after case, before shift: $1"
#	shift
#	echo "  arg 1 after shift: $1"
#done

echo "flag: $flag"
echo "barg: $barg"
echo "carg: $carg"
echo "remaining args: $*"

>Fix:
	The simplest fix, and the simplest resulting code would be to
	remove the shifts inside the case statement, so that all of the
	shifting is done by the single shift preceding the case statement.
	However, that would still be subject to a somewhat more subtle
	failure: the command line "cmd -b -a -c cc" would erroneously
	report that the -a flag had been set (because the arguments of
	options are processed by the case statement), instead of taking
	the -a as the argument of -b.  One way of coding the script that
	works correctly is shown in the script above, using while instead
	of for.  I've used an approach like that in this patch for the man
	page:

--- getopt.1.orig	Wed Dec  1 05:41:34 1999
+++ getopt.1	Thu Jun  1 20:01:38 2000
@@ -48,23 +48,23 @@
 if [ $? != 0 ]; then
 	echo 'Usage: ...'
 	exit 2
 fi
 set \-\- $args
-for i; do
-	shift
-	case "$i"; in
+while [ $# > 0 ]; do
+	case "$1"; in
 		\-a|\-b)
-			flag=$i
+			flag=$1
 			;;
 		\-c)
-			carg=$1; shift
+			carg=$2; shift
 			;;
 		\-\-)
-			break
+			shift; break
 			;;
 	esac
+	shift
 done
 .Ed
 .Pp
 This code will accept any of the following as equivalent:
 .Pp
>Release-Note:
>Audit-Trail:
>Unformatted: