Subject: lib/17248: getopt(3) doesn't error an unexpected '-' option
To: None <gnats-bugs@gnats.netbsd.org>
From: None <david@l8s.co.uk>
List: netbsd-bugs
Date: 06/13/2002 18:08:03
>Number:         17248
>Category:       lib
>Synopsis:       getopt(3) doesn't error "ls -l-"
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jun 13 10:09:00 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator:     David Laight
>Release:        NetBSD 1.5ZC
>Organization:
	dis
>Environment:
System: NetBSD snowdrop 1.5ZC NetBSD 1.5ZC (GENERIC) #10: Thu May 16 12:50:04 BST 2002 dsl@snowdrop:/oldroot/usr/bsd-current/src/sys/arch/i386/compile/GENERIC i386
Architecture: i386
Machine: i386
>Description:
	getopt(3) returns -1 if it finds an unexpected '-' option.
	This causes the program to use that entire option string
	as an argument.
>How-To-Repeat:
	Pass a '-' option to a program that doesn't expect it.

	$ : >-l-
	$ ls -l-
	-rw-r--r--  1 dsl  wheel  0 Jun 13 11:21 -l-

	This ought to generate the usage message from ls (because '-'
	if one of the few options it doesn't have).
	However you get the output of 'ls -l -- -l-'

>Fix:
	Apply either of the following patches.
	The first just fixes the above problem, the second also reduces
	the chances that a program will get incorrect answers after
	resetting optind and/or argv.
	I've also applied a cosmetic change to the code that
	determines optarg in order to made it more readable.

Index: getopt.c
===================================================================
RCS file: /cvsroot/basesrc/lib/libc/stdlib/getopt.c,v
retrieving revision 1.21
diff -u -r1.21 getopt.c
--- getopt.c	2001/04/24 09:07:43	1.21
+++ getopt.c	2002/06/13 16:42:52
@@ -83,12 +83,14 @@
 
 	if (optreset || !*place) {		/* update scanning pointer */
 		optreset = 0;
-		if (optind >= nargc || *(place = nargv[optind]) != '-') {
+		/* for SYSV compatibility treat '-' as not an option */
+		if (optind >= nargc || *(place = nargv[optind]) != '-' ||
+		    !place[1] ) {
 			place = EMSG;
 			return (-1);
 		}
-		if (place[1] && *++place == '-'	/* found "--" */
-		    && place[1] == '\0') {
+		/* "--" => end of options */
+		if (*++place == '-' && place[1] == '\0') {
 			++optind;
 			place = EMSG;
 			return (-1);
@@ -96,12 +98,6 @@
 	}					/* option letter okay? */
 	if ((optopt = (int)*place++) == (int)':' ||
 	    !(oli = strchr(ostr, optopt))) {
-		/*
-		 * if the user didn't specify '-' as an option,
-		 * assume it means -1.
-		 */
-		if (optopt == (int)'-')
-			return (-1);
 		if (!*place)
 			++optind;
 		if (opterr && *ostr != ':')

Index: getopt.c
===================================================================
RCS file: /cvsroot/basesrc/lib/libc/stdlib/getopt.c,v
retrieving revision 1.21
diff -u -r1.21 getopt.c
--- getopt.c	2001/04/24 09:07:43	1.21
+++ getopt.c	2002/06/13 16:44:22
@@ -61,8 +61,8 @@
 	optreset;		/* reset getopt */
 char	*optarg;		/* argument associated with option */
 
-#define	BADCH	(int)'?'
-#define	BADARG	(int)':'
+#define	BADCH	'?'
+#define	BADARG	':'
 #define	EMSG	""
 
 /*
@@ -75,33 +75,43 @@
 	char * const nargv[];
 	const char *ostr;
 {
 	static char *place = EMSG;		/* option letter processing */
+	static char old_optind = 1;
+	static char * const *old_argv = 0;
 	char *oli;				/* option letter list index */
 
 	_DIAGASSERT(nargv != NULL);
 	_DIAGASSERT(ostr != NULL);
 
+	/* If this isn't a continuation of the last call
+	   ensure we don't continue parsing an old option string. */
+	if (old_optind != optind || nargv != old_argv)
+	    place = EMSG;
+
 	if (optreset || !*place) {		/* update scanning pointer */
 		optreset = 0;
-		if (optind >= nargc || *(place = nargv[optind]) != '-') {
+		/* for SYSV compatibility treat '-' as not an option */
+		if (optind >= nargc || *(place = nargv[optind]) != '-' ||
+		    !place[1] ) {
 			place = EMSG;
 			return (-1);
 		}
-		if (place[1] && *++place == '-'	/* found "--" */
-		    && place[1] == '\0') {
+		/* "--" => end of options */
+		if (*++place == '-' && place[1] == '\0') {
 			++optind;
 			place = EMSG;
 			return (-1);
 		}
-	}					/* option letter okay? */
-	if ((optopt = (int)*place++) == (int)':' ||
-	    !(oli = strchr(ostr, optopt))) {
-		/*
-		 * if the user didn't specify '-' as an option,
-		 * assume it means -1.
-		 */
-		if (optopt == (int)'-')
-			return (-1);
+	}
+
+	/* Save these to detect (most) cases when the user resets
+	   optind in order to process a different set of arguments
+	   (basically makes us kill the saved 'place' next time around). */
+	old_optind = optind;
+	old_argv = nargv;
+
+	/* option letter okay? */
+	if ((optopt = *place++) == ':' || !(oli = strchr(ostr, optopt))) {
 		if (!*place)
 			++optind;
 		if (opterr && *ostr != ':')
@@ -118,7 +128,9 @@
 	else {					/* need an argument */
 		if (*place)			/* no white space */
 			optarg = place;
-		else if (nargc <= ++optind) {	/* no arg */
+		else if (nargc > ++optind)	/* white space */
+			optarg = nargv[optind];
+		else {				/* no arg */
 			place = EMSG;
 			if (*ostr == ':')
 				return (BADARG);
@@ -128,8 +140,6 @@
 				    getprogname(), optopt);
 			return (BADCH);
 		}
-	 	else				/* white space */
-			optarg = nargv[optind];
 		place = EMSG;
 		++optind;
 	}
>Release-Note:
>Audit-Trail:
>Unformatted: