34992: POSIX fix for readonly variables.

With POSIXBUILTINS, variables can be marked readonly if unset.
Also, variables can't have the readonly flag removed.

5 files changed, 69 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index a5987b764..2d2fb1b5a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2015-04-29  Peter Stephenson  <p.stephenson@samsung.com>
+
+	* 34992: Doc/Zsh/builtins.yo, Src/builtin.c, Src/params.c,
+	Test/B02typeset.ztst: With POSXIBUILTINS, parameters can be
+	marked readonly if unset and in any case can't subsequently be
+	marked not readonly.
+
 2015-04-28  Peter Stephenson  <p.w.stephenson@ntlworld.com>
 
 	* 34989: Src/exec.c: AUTOCD needs to pass -- to cd to avoid
diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index dd5a80fe8..985d19e11 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -1933,6 +1933,13 @@ item(tt(-r))(
 The given var(name)s are marked readonly.  Note that if var(name) is a
 special parameter, the readonly attribute can be turned on, but cannot then
 be turned off.
+
+If the tt(POSIX_BUILTINS) option is set, the readonly attribute is
+more restrictive: unset variables can be marked readonly and cannot then
+be set; furthermore, the readonly attribute cannot be removed from any
+variable.  Note that in zsh (unlike other shells) it is still possible
+to create a local variable of the same name as this is considered a
+different variable (though this variable, too, can be marked readonly).
 )
 item(tt(-t))(
 Tags the named parameters.  Tags have no special meaning to the shell.
diff --git a/Src/builtin.c b/Src/builtin.c
index de0101405..0a57489ea 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -1931,8 +1931,12 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
      * locallevel as an unset one we use the pm struct anyway: that's
      * handled in createparam().  Here we just avoid using it for the
      * present tests if it's unset.
+     *
+     * POSIXBUILTINS horror: we need to retain the 'readonly' flag
+     * of an unset parameter.
      */
-    usepm = pm && !(pm->node.flags & PM_UNSET);
+    usepm = pm && (!(pm->node.flags & PM_UNSET) ||
+		   (isset(POSIXBUILTINS) && (pm->node.flags & PM_READONLY)));
 
     /*
      * We need to compare types with an existing pm if special,
@@ -2032,6 +2036,20 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
     else if (newspecial != NS_NONE && strcmp(pname, "SECONDS") == 0)
 	newspecial = NS_SECONDS;
 
+    if (isset(POSIXBUILTINS)) {
+	/*
+	 * Stricter rules about retaining readonly attribute in this case.
+	 */
+	if ((on & PM_READONLY) && (!usepm || (pm->node.flags & PM_UNSET)) &&
+	    !value)
+	    on |= PM_UNSET;
+	else if (usepm && (pm->node.flags & PM_READONLY) &&
+		 !(on & PM_READONLY)) {
+	    zerr("read-only variable: %s", pm->node.nam);
+	    return NULL;
+	}
+    }
+
     /*
      * A parameter will be local if
      * 1. we are re-using an existing local parameter
@@ -2078,9 +2096,15 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	}
 	if (usepm == 2)		/* do not change the PM_UNSET flag */
 	    pm->node.flags = (pm->node.flags | (on & ~PM_READONLY)) & ~off;
-	else
+	else {
+	    /*
+	     * Keep unset if using readonly in POSIX mode.
+	     */
+	    if (!(on & PM_READONLY) || !isset(POSIXBUILTINS))
+		off |= PM_UNSET;
 	    pm->node.flags = (pm->node.flags |
-			      (on & ~PM_READONLY)) & ~(off | PM_UNSET);
+			      (on & ~PM_READONLY)) & ~off;
+	}
 	if (on & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z)) {
 	    if (typeset_setwidth(cname, pm, ops, on, 0))
 		return NULL;
@@ -2256,7 +2280,12 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	 * readonly flag
 	 */
 	pm = createparam(pname, on & ~PM_READONLY);
-	DPUTS(!pm, "BUG: parameter not created");
+	if (!pm) {
+	    if (on & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z |
+		      PM_INTEGER | PM_EFLOAT | PM_FFLOAT))
+		zerrnam(cname, "can't change variable attribute: %s", pname);
+	    return NULL;
+	}
 	if (on & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z)) {
 	    if (typeset_setwidth(cname, pm, ops, on, 0))
 		return NULL;
diff --git a/Src/params.c b/Src/params.c
index e8a90104c..d53b6ca7e 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -874,10 +874,14 @@ createparam(char *name, int flags)
 	DPUTS(oldpm && oldpm->level > locallevel,
 	      "BUG: old local parameter not deleted");
 	if (oldpm && (oldpm->level == locallevel || !(flags & PM_LOCAL))) {
+	    if (isset(POSIXBUILTINS) && (oldpm->node.flags & PM_READONLY)) {
+		zerr("read-only variable: %s", name);
+		return NULL;
+	    }
 	    if (!(oldpm->node.flags & PM_UNSET) || (oldpm->node.flags & PM_SPECIAL)) {
 		oldpm->node.flags &= ~PM_UNSET;
 		if ((oldpm->node.flags & PM_SPECIAL) && oldpm->ename) {
-		    Param altpm = 
+		    Param altpm =
 			(Param) paramtab->getnode(paramtab, oldpm->ename);
 		    if (altpm)
 			altpm->node.flags &= ~PM_UNSET;
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 51ebc6535..f4fb8ecb9 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -468,3 +468,20 @@
 0:retying arrays to same array works
 >foo bar
 >goo car
+
+ (
+   setopt POSIXBUILTINS
+   readonly pbro
+   print ${+pbro} >&2
+   (typeset pbro=3)
+   (pbro=4)
+   typeset -r pbro        # idempotent (no error)...
+   print ${+pbro} >&2     # ...so still readonly...
+   typeset +r pbro        # ...can't turn it off
+ )
+1:Readonly with POSIX_BUILTINS
+?0
+?(eval):5: read-only variable: pbro
+?(eval):6: read-only variable: pbro
+?0
+?(eval):9: read-only variable: pbro