[PATCH] getopts: fix returned value on missing option argument

[Martijn Dekker <martijn@xxxxxxxx>]

Problem:

$ zsh -c 'while getopts x: opt; do echo "$opt"; done' dummy -x
zsh:1: argument expected after -x option
:

Expected output: the ':' at the end should be a '?'. The opt variable 
should only be set to ':' in "quiet" mode, i.e. if the option string 
starts with a ':' (so in this case would be ':x:').

Ref.: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/getopts.html
(the two bullet points following "If an option-argument is missing:").

The attached patch fixes this to match POSIX and the behaviour of every 
other shell.

- Martijn

--
modernish -- harness the shell
https://github.com/modernish/modernish

diff --git a/Src/builtin.c b/Src/builtin.c
index e863cc4bb..9b9e76c77 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5511,14 +5511,12 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun
     /* check for legality */
     if(opch == ':' || !(p = memchr(optstr, opch, lenoptstr))) {
 	p = "?";
-    err:
 	zsfree(zoptarg);
 	setsparam(var, ztrdup(p));
 	if(quiet) {
 	    zoptarg = metafy(optbuf, lenoptbuf, META_DUP);
 	} else {
-	    zwarn(*p == '?' ? "bad option: %c%c" :
-		  "argument expected after %c%c option",
+	    zwarn("bad option: %c%c",
 		  "?-+"[lenoptbuf], opch);
 	    zoptarg=ztrdup("");
 	}
@@ -5529,8 +5527,17 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun
     if(p[1] == ':') {
 	if(optcind == lenstr) {
 	    if(!args[zoptind]) {
-		p = ":";
-		goto err;
+		zsfree(zoptarg);
+		if(quiet) {
+		    setsparam(var, ztrdup(":"));
+		    zoptarg = metafy(optbuf, lenoptbuf, META_DUP);
+		} else {
+		    setsparam(var, ztrdup("?"));
+		    zoptarg = ztrdup("");
+		    zwarn("argument expected after %c%c option",
+			  "?-+"[lenoptbuf], opch);
+		}
+		return 0;
 	    }
 	    p = ztrdup(args[zoptind++]);
 	} else
diff --git a/Test/B10getopts.ztst b/Test/B10getopts.ztst
index 7eba5a4b1..72c9e209e 100644
--- a/Test/B10getopts.ztst
+++ b/Test/B10getopts.ztst
@@ -79,3 +79,20 @@
   test_getopts +x
 1:one illegal option, + variant
 >test_getopts:3: bad option: +x
+
+  set -- -x
+  OPTIND=1
+  while getopts x: opt; do
+    echo "$opt,${OPTARG:-Empty}"
+  done
+0:missing option-argument (error message mode)
+>?,Empty
+?(eval):3: argument expected after -x option
+
+  set -- -x
+  OPTIND=1
+  while getopts :x: opt; do
+    echo "$opt,${OPTARG:-Empty}"
+  done
+0:missing option-argument (quiet mode)
+>:,x