From 1bc2c74c7454bbab2c2067fdf205c07a53027f52 Mon Sep 17 00:00:00 2001
From: Johnothan King <johnothanking@protonmail.com>
Date: Sat, 25 Jul 2020 18:18:49 -0700
Subject: [PATCH] Fix how unrecognized options are handled in 'sleep' and
 'suspend' (#93)

When a builtin is given an unrecognized option, the usage information
for that builtin should be shown as 'Usage: builtin-name options'. The
sleep and suspend builtins were an exception to this. 'suspend' would
not show usage information and sleep wouldn't exit on error:

$ suspend -e
/usr/bin/ksh: suspend: -e: unknown option
$ time sleep -e 1
sleep: -e: unknown option

real	0m1.00s
user	0m0.00s
sys	0m0.00s

src/cmd/ksh93/bltins/sleep.c:
- Show usage information and exit when sleep is given an unknown
  option. This bugfix was backported from ksh2020: https://github.com/att/ast/pull/1024

src/cmd/ksh93/bltins/trap.c:
- Use the normal method of parsing options with optget to fix the
  suspend builtin's test failure.

src/cmd/ksh93/tests/builtins.sh:
- Add the ksh2020 regression test for getting the usage information
  of each builtin. Enable all /opt/ast/bin builtins in a subshell
  since those should be tested as well (aside from getconf and uname
  because those builtins fallback to the real commands on error).
---
 NEWS                            |  4 ++++
 src/cmd/ksh93/bltins/sleep.c    |  2 ++
 src/cmd/ksh93/bltins/trap.c     | 15 +++++++++++++--
 src/cmd/ksh93/tests/builtins.sh | 11 +++++++++++
 4 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 3ea2ded9d..b82989706 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
   based on the incorrect assumption the IFS would never be larger
   than a single byte.
 
+- Fixed a bug that caused the sleep builtin to continue after being given
+  an unrecognized option. 'sleep -: 1' will now show a usage message and
+  exit instead of sleep for one second.
+
 2020-07-23:
 
 - Fixed an infinite loop that could occur when ksh is the system's /bin/sh.
diff --git a/src/cmd/ksh93/bltins/sleep.c b/src/cmd/ksh93/bltins/sleep.c
index c34f3db53..15df5b1a7 100644
--- a/src/cmd/ksh93/bltins/sleep.c
+++ b/src/cmd/ksh93/bltins/sleep.c
@@ -62,6 +62,8 @@ int	b_sleep(register int argc,char *argv[],Shbltin_t *context)
 			errormsg(SH_DICT,ERROR_usage(2), "%s", opt_info.arg);
 			break;
 	}
+	if(error_info.errors)
+		errormsg(SH_DICT, ERROR_usage(2), "%s", optusage(NULL));
 	argv += opt_info.index;
 	if(cp = *argv)
 	{
diff --git a/src/cmd/ksh93/bltins/trap.c b/src/cmd/ksh93/bltins/trap.c
index 1e19c2594..974ac5de4 100644
--- a/src/cmd/ksh93/bltins/trap.c
+++ b/src/cmd/ksh93/bltins/trap.c
@@ -249,8 +249,19 @@ endopts:
 int	b_suspend(int argc,char *argv[],Shbltin_t *context)
 {
 	NOT_USED(argc);
-	if(optget(argv, sh_optsuspend))	/* no options supported (except AST --man, etc.) */
-		errormsg(SH_DICT, ERROR_exit(2), "%s", opt_info.arg);
+
+	int n;
+	while((n = optget(argv, sh_optsuspend))) switch(n)
+	{
+		case ':':
+			errormsg(SH_DICT,2, "%s", opt_info.arg);
+			break;
+		case '?':
+			errormsg(SH_DICT,ERROR_usage(2), "%s", opt_info.arg);
+			break;
+	}
+	if(error_info.errors)	/* no options supported (except AST --man, etc.) */
+		errormsg(SH_DICT,ERROR_usage(2),"%s", optusage((char*)0));
 	if(argv[opt_info.index])	/* no operands supported */
 		errormsg(SH_DICT, ERROR_exit(2), e_toomanyops);
 	if(sh_isoption(SH_LOGIN_SHELL))
diff --git a/src/cmd/ksh93/tests/builtins.sh b/src/cmd/ksh93/tests/builtins.sh
index 46a880b9d..da0291d2e 100755
--- a/src/cmd/ksh93/tests/builtins.sh
+++ b/src/cmd/ksh93/tests/builtins.sh
@@ -885,5 +885,16 @@ fi
 EOF
 "$SHELL" -i "$sleepsig" 2> /dev/null || err_exit "'sleep -s' doesn't work with intervals of more than 30 seconds"
 
+# ==========
+# Builtins should handle unrecognized options correctly
+(
+	builtin $(builtin -l | awk -F "/" '/\/opt/ {print $5}') # Load all /opt/ast/bin builtins
+	for name in $(builtin -l | grep -Ev '(echo|/opt|test|true|false|getconf|uname|\[|:)'); do
+	    actual="$($name --this-option-does-not-exist 2>&1)"
+	    expect="Usage: $name"
+	    [[ $actual =~ $expect ]] || err_exit "$name should show usage info on unrecognized options (expected $(printf '%q' "$expect"), got $(printf '%q' "$actual"))"
+	done
+)
+
 # ======
 exit $((Errors<125?Errors:125))