• Implement http://austingroupbugs.net/view.php?id=367 and align things

a bit more with POSIX and the other shells

I considered http://austingroupbugs.net/view.php?id=253 but the use
of bi_errorf() is interesting, especially as it’s often enough a
noreturn function, and funnily enough, 'cd -P /foo' returns 0 while
'chdir -P /foo' fails (so idk where to put -e)…

check.t

@@ -1,4 +1,4 @@
-# $MirOS: src/bin/mksh/check.t,v 1.437 2011/03/26 21:46:00 tg Exp $
+# $MirOS: src/bin/mksh/check.t,v 1.438 2011/03/27 18:50:02 tg Exp $
 # $OpenBSD: bksl-nl.t,v 1.2 2001/01/28 23:04:56 niklas Exp $
 # $OpenBSD: history.t,v 1.5 2001/01/28 23:04:56 niklas Exp $
 # $OpenBSD: read.t,v 1.3 2003/03/10 03:48:16 david Exp $
@@ -25,7 +25,7 @@
 # http://www.research.att.com/~gsf/public/ifs.sh
 
 expected-stdout:
-	@(#)MIRBSD KSH R39 2011/03/26
+	@(#)MIRBSD KSH R39 2011/03/27
 description:
 	Check version of shell.
 stdin:
@@ -4679,6 +4679,65 @@ stdin:
 expected-stdout:
 	ok
 ---
+name: readonly-0
+description:
+	Ensure readonly is honoured for assignments and unset
+stdin:
+	"$__progname" -c 'u=x; echo $? $u .' || echo aborted, $?
+	echo =
+	"$__progname" -c 'readonly u; u=x; echo $? $u .' || echo aborted, $?
+	echo =
+	"$__progname" -c 'u=x; readonly u; unset u; echo $? $u .' || echo aborted, $?
+expected-stdout:
+	0 x .
+	=
+	aborted, 2
+	=
+	1 x .
+expected-stderr-pattern:
+	/read *only/
+---
+name: readonly-1
+description:
+	http://austingroupbugs.net/view.php?id=367 for export
+stdin:
+	"$__progname" -c 'readonly foo; export foo=a; echo $?' || echo aborted, $?
+expected-stdout:
+	aborted, 2
+expected-stderr-pattern:
+	/read *only/
+---
+name: readonly-2a
+description:
+	Check that getopts works as intended, for readonly-2b to be valid
+stdin:
+	"$__progname" -c 'set -- -a b; getopts a c; echo $? $c .; getopts a c; echo $? $c .' || echo aborted, $?
+expected-stdout:
+	0 a .
+	1 ? .
+---
+name: readonly-2b
+description:
+	http://austingroupbugs.net/view.php?id=367 for getopts
+stdin:
+	"$__progname" -c 'readonly c; set -- -a b; getopts a c; echo $? $c .' || echo aborted, $?
+expected-stdout:
+	2 .
+expected-stderr-pattern:
+	/read *only/
+---
+name: readonly-3
+description:
+	http://austingroupbugs.net/view.php?id=367 for read
+stdin:
+	echo x | "$__progname" -c 'read s; echo $? $s .' || echo aborted, $?
+	echo y | "$__progname" -c 'readonly s; read s; echo $? $s .' || echo aborted, $?
+expected-stdout:
+	0 x .
+	2 .
+expected-stderr-pattern:
+	/read *only/
+---
 name: syntax-1
 description:
 	Check that lone ampersand is a syntax error

funcs.c

@@ -38,7 +38,7 @@
 #endif
 #endif
 
-__RCSID("$MirOS: src/bin/mksh/funcs.c,v 1.181 2011/03/27 01:30:36 tg Exp $");
+__RCSID("$MirOS: src/bin/mksh/funcs.c,v 1.182 2011/03/27 18:50:04 tg Exp $");
 
 #if HAVE_KILLPG
 /*
@@ -1454,7 +1454,7 @@ c_getopts(const char **wp)
 	vq = global(var);
 	/* Error message already printed (integer, readonly) */
 	if (!setstr(vq, buf, KSH_RETURN_ERROR))
-		rv = 1;
+		rv = 2;
 	if (Flag(FEXPORT))
 		typeset(var, EXPORT, 0, 0, 0);
 
@@ -1917,7 +1917,7 @@ c_read(const char **wp)
 			shf_flush(shf);
 			bi_errorf("%s: %s", *wp, "is read only");
 			afree(wpalloc, ATEMP);
-			return (1);
+			return (2);
 		}
 		if (Flag(FEXPORT))
 			typeset(*wp, EXPORT, 0, 0, 0);
@@ -2187,7 +2187,7 @@ int
 c_unset(const char **wp)
 {
 	const char *id;
-	int optc;
+	int optc, rv = 0;
 	bool unset_var = true;
 
 	while ((optc = ksh_getopt(wp, &builtin_opt, "fv")) != -1)
@@ -2199,7 +2199,8 @@ c_unset(const char **wp)
 			unset_var = true;
 			break;
 		case '?':
-			return (1);
+			/*XXX not reached due to GF_ERROR */
+			return (2);
 		}
 	wp += builtin_opt.optind;
 	for (; (id = *wp) != NULL; wp++)
@@ -2222,14 +2223,15 @@ c_unset(const char **wp)
 			afree(cp, ATEMP);
 
 			if ((vp->flag&RDONLY)) {
-				bi_errorf("%s: %s", vp->name, "is read only");
+				warningf(true, "%s: %s", vp->name,
-				return (1);
+				    "is read only");
-			}
+				rv = 1;
-			unset(vp, optc);
+			} else
+				unset(vp, optc);
 		} else
 			/* unset function */
 			define(id, NULL);
-	return (0);
+	return (rv);
 }
 
 static void

main.c

@@ -33,7 +33,7 @@
 #include <locale.h>
 #endif
 
-__RCSID("$MirOS: src/bin/mksh/main.c,v 1.186 2011/03/26 19:43:47 tg Exp $");
+__RCSID("$MirOS: src/bin/mksh/main.c,v 1.187 2011/03/27 18:50:05 tg Exp $");
 
 extern char **environ;
 
@@ -999,23 +999,59 @@ tty_close(void)
 }
 
 /* A shell error occurred (eg, syntax error, etc.) */
+
+#define VWARNINGF_ERRORPREFIX	1
+#define VWARNINGF_FILELINE	2
+#define VWARNINGF_BUILTIN	4
+#define VWARNINGF_INTERNAL	8
+
+static void MKSH_A_FORMAT(printf, 2, 0)
+vwarningf(unsigned int flags, const char *fmt, va_list ap)
+{
+	if (*fmt != 1) {
+		if (flags & VWARNINGF_INTERNAL)
+			shf_fprintf(shl_out, "internal error: ");
+		if (flags & VWARNINGF_ERRORPREFIX)
+			error_prefix(tobool(flags & VWARNINGF_FILELINE));
+		if ((flags & VWARNINGF_BUILTIN) &&
+		    /* not set when main() calls parse_args() */
+		    builtin_argv0 && builtin_argv0 != kshname)
+			shf_fprintf(shl_out, "%s: ", builtin_argv0);
+		shf_vfprintf(shl_out, fmt, ap);
+		shf_putchar('\n', shl_out);
+	}
+	shf_flush(shl_out);
+}
+
+void
+errorfx(int rc, const char *fmt, ...)
+{
+	va_list va;
+
+	exstat = rc;
+
+	/* debugging: note that stdout not valid */
+	shl_stdout_ok = false;
+
+	va_start(va, fmt);
+	vwarningf(VWARNINGF_ERRORPREFIX | VWARNINGF_FILELINE, fmt, va);
+	va_end(va);
+	unwind(LERROR);
+}
+
 void
 errorf(const char *fmt, ...)
 {
 	va_list va;
 
+	exstat = 1;
+
 	/* debugging: note that stdout not valid */
 	shl_stdout_ok = false;
 
-	exstat = 1;
+	va_start(va, fmt);
-	if (*fmt != 1) {
+	vwarningf(VWARNINGF_ERRORPREFIX | VWARNINGF_FILELINE, fmt, va);
-		error_prefix(true);
+	va_end(va);
-		va_start(va, fmt);
-		shf_vfprintf(shl_out, fmt, va);
-		va_end(va);
-		shf_putchar('\n', shl_out);
-	}
-	shf_flush(shl_out);
 	unwind(LERROR);
 }
 
@@ -1025,12 +1061,10 @@ warningf(bool fileline, const char *fmt, ...)
 {
 	va_list va;
 
-	error_prefix(fileline);
 	va_start(va, fmt);
-	shf_vfprintf(shl_out, fmt, va);
+	vwarningf(VWARNINGF_ERRORPREFIX | (fileline ? VWARNINGF_FILELINE : 0),
+	    fmt, va);
 	va_end(va);
-	shf_putchar('\n', shl_out);
-	shf_flush(shl_out);
 }
 
 /*
@@ -1046,17 +1080,12 @@ bi_errorf(const char *fmt, ...)
 	shl_stdout_ok = false;
 
 	exstat = 1;
-	if (*fmt != 1) {
+
-		error_prefix(true);
+	va_start(va, fmt);
-		/* not set when main() calls parse_args() */
+	vwarningf(VWARNINGF_ERRORPREFIX | VWARNINGF_FILELINE |
-		if (builtin_argv0 && builtin_argv0 != kshname)
+	    VWARNINGF_BUILTIN, fmt, va);
-			shf_fprintf(shl_out, "%s: ", builtin_argv0);
+	va_end(va);
-		va_start(va, fmt);
+
-		shf_vfprintf(shl_out, fmt, va);
-		va_end(va);
-		shf_putchar('\n', shl_out);
-	}
-	shf_flush(shl_out);
 	/*
 	 * POSIX special builtins and ksh special builtins cause
 	 * non-interactive shells to exit.
@@ -1069,22 +1098,13 @@ bi_errorf(const char *fmt, ...)
 }
 
 /* Called when something that shouldn't happen does */
-void
-internal_verrorf(const char *fmt, va_list ap)
-{
-	shf_fprintf(shl_out, "internal error: ");
-	shf_vfprintf(shl_out, fmt, ap);
-	shf_putchar('\n', shl_out);
-	shf_flush(shl_out);
-}
-
 void
 internal_errorf(const char *fmt, ...)
 {
 	va_list va;
 
 	va_start(va, fmt);
-	internal_verrorf(fmt, va);
+	vwarningf(VWARNINGF_INTERNAL, fmt, va);
 	va_end(va);
 	unwind(LERROR);
 }
@@ -1095,7 +1115,7 @@ internal_warningf(const char *fmt, ...)
 	va_list va;
 
 	va_start(va, fmt);
-	internal_verrorf(fmt, va);
+	vwarningf(VWARNINGF_INTERNAL, fmt, va);
 	va_end(va);
 }
 

sh.h

@@ -154,9 +154,9 @@
 #endif
 
 #ifdef EXTERN
-__RCSID("$MirOS: src/bin/mksh/sh.h,v 1.455 2011/03/27 01:30:38 tg Exp $");
+__RCSID("$MirOS: src/bin/mksh/sh.h,v 1.456 2011/03/27 18:50:05 tg Exp $");
 #endif
-#define MKSH_VERSION "R39 2011/03/26"
+#define MKSH_VERSION "R39 2011/03/27"
 
 #ifndef MKSH_INCLUDES_ONLY
 
@@ -1642,14 +1642,16 @@ void cleanup_proc_env(void);
 void errorf(const char *, ...)
     MKSH_A_NORETURN
     MKSH_A_FORMAT(printf, 1, 2);
+void errorfx(int, const char *, ...)
+    MKSH_A_NORETURN
+    MKSH_A_FORMAT(printf, 2, 3);
 void warningf(bool, const char *, ...)
     MKSH_A_FORMAT(printf, 2, 3);
 void bi_errorf(const char *, ...)
     MKSH_A_FORMAT(printf, 1, 2);
 #define errorfz()	errorf("\1")
+#define errorfxz(rc)	errorfx((rc), "\1")
 #define bi_errorfz()	bi_errorf("\1")
-void internal_verrorf(const char *, va_list)
-    MKSH_A_FORMAT(printf, 1, 0);
 void internal_errorf(const char *, ...)
     MKSH_A_NORETURN
     MKSH_A_FORMAT(printf, 1, 2);

var.c

@@ -26,7 +26,7 @@
 #include <sys/sysctl.h>
 #endif
 
-__RCSID("$MirOS: src/bin/mksh/var.c,v 1.118 2011/03/13 01:20:25 tg Exp $");
+__RCSID("$MirOS: src/bin/mksh/var.c,v 1.119 2011/03/27 18:50:06 tg Exp $");
 
 /*
  * Variables
@@ -407,7 +407,7 @@ setstr(struct tbl *vq, const char *s, int error_ok)
 	if ((vq->flag & RDONLY) && !no_ro_check) {
 		warningf(true, "%s: %s", vq->name, "is read only");
 		if (!error_ok)
-			errorfz();
+			errorfxz(2);
 		return (0);
 	}
 	if (!(vq->flag&INTEGER)) { /* string dest */
@@ -758,7 +758,7 @@ typeset(const char *var, Tflag set, Tflag clr, int field, int base)
 	if ((vpbase->flag&RDONLY) &&
 	    (val || clr || (set & ~EXPORT)))
 		/* XXX check calls - is error here ok by POSIX? */
-		errorf("%s: %s", tvar, "is read only");
+		errorfx(2, "%s: %s", tvar, "is read only");
 	afree(tvar, ATEMP);
 
 	/* most calls are with set/clr == 0 */
@@ -1313,7 +1313,7 @@ set_array(const char *var, bool reset, const char **vals)
 
 	/* Note: AT&T ksh allows set -A but not set +A of a read-only var */
 	if ((vp->flag&RDONLY))
-		errorf("%s: %s", var, "is read only");
+		errorfx(2, "%s: %s", var, "is read only");
 	/* This code is quite non-optimal */
 	if (reset)
 		/* trash existing values and attributes */