From 1b5bc1802a97010a0225bd7dba55001475334966 Mon Sep 17 00:00:00 2001
From: Johnothan King <johnothanking@protonmail.com>
Date: Mon, 29 Jun 2020 11:09:20 -0700
Subject: [PATCH] Fix the readonly builtin's scope in functions (#51)

* Fix the readonly builtin's scope in functions

This bug was first reported at https://github.com/att/ast/issues/881

'tdata.sh->prefix' is only set to the correct value when
'b_readonly' is called as 'export', which breaks 'readonly' in
functions because the correct scope isn't set. As a result, the
following example will only print a newline:

$ function show_bar { readonly foo=bar; echo $foo; }; show_bar

The fix is to move the required code out of the if statement for
'export', as it needs to be run for 'readonly' as well. This bugfix
is from https://github.com/att/ast/pull/906

src/cmd/ksh93/bltins/typeset.c:
- Set 'tdata.sh->prefix' to the correct value, otherwise 'readonly'
  uses the wrong scope.

src/cmd/ksh93/tests/builtins.sh:
- Add the regression test from ksh2020, modified to run in a
  subshell.

src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Add documentation of 'readonly' vs. 'typeset -r' difference:
  'readonly' does not create a function-local scope.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
---
 NEWS                            |  4 ++++
 src/cmd/ksh93/bltins/typeset.c  |  6 ++----
 src/cmd/ksh93/data/builtins.c   |  5 ++++-
 src/cmd/ksh93/sh.1              |  6 ++++++
 src/cmd/ksh93/tests/builtins.sh | 12 ++++++++++++
 5 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index bc42c4e3f..54bf61d7b 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 - Unsetting an array that was turned into a compound variable will no
   longer cause silent memory corruption.
 
+- Variables created with 'readonly' in functions are now set to the
+  specified value instead of nothing. Note that 'readonly' does not
+  create a function-local scope, unlike 'typeset -r' which does.
+
 2020-06-26:
 
 - Changing to a directory that has a name starting with a '.' will no
diff --git a/src/cmd/ksh93/bltins/typeset.c b/src/cmd/ksh93/bltins/typeset.c
index 7493c8f94..d84bdf49b 100644
--- a/src/cmd/ksh93/bltins/typeset.c
+++ b/src/cmd/ksh93/bltins/typeset.c
@@ -125,11 +125,9 @@ int    b_readonly(int argc,char *argv[],Shbltin_t *context)
 	}
 #endif
 	else
-	{
 		flag = (NV_ASSIGN|NV_EXPORT|NV_IDENT);
-		if(!tdata.sh->prefix)
-			tdata.sh->prefix = "";
-	}
+	if(!tdata.sh->prefix)
+		tdata.sh->prefix = "";
 	return(setall(argv,flag,tdata.sh->var_tree, &tdata));
 }
 
diff --git a/src/cmd/ksh93/data/builtins.c b/src/cmd/ksh93/data/builtins.c
index 5b3c8fb57..42b0399ab 100644
--- a/src/cmd/ksh93/data/builtins.c
+++ b/src/cmd/ksh93/data/builtins.c
@@ -1360,7 +1360,7 @@ USAGE_LICENSE
 ;
 
 const char sh_optreadonly[] =
-"[-1c?\n@(#)$Id: readonly (AT&T Research) 2008-06-16 $\n]"
+"[-1c?\n@(#)$Id: readonly (AT&T Research/ksh93) 2020-06-28 $\n]"
 USAGE_LICENSE
 "[+NAME?readonly - set readonly attribute on variables]"
 "[+DESCRIPTION?\breadonly\b sets the readonly attribute on each of "
@@ -1368,6 +1368,9 @@ USAGE_LICENSE
 	"values from being changed.  If \b=\b\avalue\a is specified, "
 	"the variable \aname\a is set to \avalue\a before the variable "
 	"is made readonly.]"
+"[+?Unlike \btypeset -r\b, \breadonly\b does not create a function-local "
+	"scope and the given \aname\as are marked globally read-only by "
+	"default.]"
 "[+?Within a type definition, if the value is not specified, then a "
 	"value must be specified when creating each instance of the type "
         "and the value is readonly for each instance.]"
diff --git a/src/cmd/ksh93/sh.1 b/src/cmd/ksh93/sh.1
index 9b335e568..1143fe4ca 100644
--- a/src/cmd/ksh93/sh.1
+++ b/src/cmd/ksh93/sh.1
@@ -6752,6 +6752,12 @@ are marked
 readonly and these
 names cannot be changed
 by subsequent assignment.
+Unlike
+.B typeset\ -r ,
+.B readonly
+does not create a function-local scope and the given
+.IR vname s
+are marked globally read-only by default.
 When defining a type, if the value of a readonly sub-variable is not defined
 the value is required when creating each instance.
 .TP
diff --git a/src/cmd/ksh93/tests/builtins.sh b/src/cmd/ksh93/tests/builtins.sh
index f75c4713d..ab2fb699e 100755
--- a/src/cmd/ksh93/tests/builtins.sh
+++ b/src/cmd/ksh93/tests/builtins.sh
@@ -726,5 +726,17 @@ chmod is a tracked alias for $(whence -p chmod)"
 	cd ../.bar
 ) || err_exit 'cd ../.bar when ../.bar exists should not fail'
 
+# ======
+# 'readonly' should set the correct scope when creating variables in functions
+unset foo
+(
+	function test_func
+	{
+		readonly foo="bar"
+		[[ "$foo" = "bar" ]] || err_exit "readonly variable is not assigned a value inside functions"
+	}
+	test_func
+)
+
 # ======
 exit $((Errors<125?Errors:125))