Re: zsh4.0.1-pre4 : backticks fail if SHLVL nonexistant

Re: zsh4.0.1-pre4 : backticks fail if SHLVL nonexistant

[Peter Stephenson]

> I've downloaded and built zsh4.0.1-pre4.  I was finding that any attempt
> to use backtick-substitution was resulting in a core dump.  I rebuilt
> with -g and tracked it down to a null pointer 'pm' in setiparam, for an
> attempt to set SHLVL from line 2316 of exec.c.  Unfortunately, I had
> SHLVL being unset by a line in my .zshenv.  (In order to reduce the
> environment to a clean initial state before adding new defintions, I
> unset everything except for a defined list of variables at the start of
> .zshenv.  Previously, SHLVL wasn't one ofthem.  It worked OK with zsh
> v3.1.5).

I can reproduce such a crash reliably with:

% unset SHLVL
% (( SHLVL-- ))
BUG: parameter not created
zsh: segmentation fault (core dumped)  ./zsh

This probably doesn't involve the line in exec.c, but it's almost certainly
the same root, so it'll give something to work on.  (The BUG message only
appears if configured with --enable-zsh-debug.)

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Unit 300, Science Park, Milton Road,
Cambridge, CB4 0XL, UK                          Tel: +44 (0)1223 392070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

PATCH: Re: zsh4.0.1-pre4 : backticks fail if SHLVL nonexistant

[Bart Schaefer]

On May 18,  2:24pm, Peter Stephenson wrote:
} Subject: Re: zsh4.0.1-pre4 : backticks fail if SHLVL nonexistant
}
} % unset SHLVL
} % (( SHLVL-- ))
} BUG: parameter not created

It's possible for createparam() to return NULL because it re-used an
existing special parameter node, rather than because it actually failed.
Probably there ought to be some kind of wrapper around createparam() for
this, but for now we can just test it where necessary.

However, that wasn't the only problem: setiparam() and setnparam() were
also bypassing the parameter's sets.ifn, which was OK only for non-special
integer parameters.

There's some stuff going on here with `outputradix' that I'm not sure is
correct.  setiparam() was ignoring the outputradix, setnparam() was using
it; maybe that's because only setnparam() gets called from the math code?
Anyway, I made them both use it, but maybe neither one should.

diff -u zsh-forge/current/Src/params.c zsh-4.0/Src/params.c
--- zsh-forge/current/Src/params.c	Tue May  1 08:37:03 2001
+++ zsh-4.0/Src/params.c	Fri May 18 14:06:10 2001
@@ -1998,17 +1998,16 @@
     if (!(v = getvalue(&vbuf, &s, 1))) {
 	if ((ss = strchr(s, '[')))
 	    *ss = '\0';
-	pm = createparam(t, ss ? PM_ARRAY : PM_INTEGER);
+	if (!(pm = createparam(t, ss ? PM_ARRAY : PM_INTEGER)))
+	    pm = (Param) paramtab->getnode(paramtab, t);
 	DPUTS(!pm, "BUG: parameter not created");
 	if (ss) {
 	    *ss = '[';
-	    v = getvalue(&vbuf, &t, 1);
-	    DPUTS(!v, "BUG: value not found for new parameter");
 	} else {
-	    pm->u.val = val;
-	    unqueue_signals();
-	    return pm;
+	    pm->ct = outputradix;
 	}
+	v = getvalue(&vbuf, &t, 1);
+	DPUTS(!v, "BUG: value not found for new parameter");
     }
     mnval.type = MN_INTEGER;
     mnval.u.l = val;
@@ -2042,20 +2041,16 @@
 	    *ss = '\0';
 	pm = createparam(t, ss ? PM_ARRAY :
 			 (val.type & MN_INTEGER) ? PM_INTEGER : PM_FFLOAT);
+	if (!pm)
+	    pm = (Param) paramtab->getnode(paramtab, t);
 	DPUTS(!pm, "BUG: parameter not created");
 	if (ss) {
 	    *ss = '[';
-	    v = getvalue(&vbuf, &t, 1);
-	    DPUTS(!v, "BUG: value not found for new parameter");
-	} else {
-	    if (val.type & MN_INTEGER) {
-		pm->ct = outputradix;
-		pm->u.val = val.u.l;
-	    } else
-		pm->u.dval = val.u.d;
-	    unqueue_signals();
-	    return pm;
+	} else if (val.type & MN_INTEGER) {
+	    pm->ct = outputradix;
 	}
+	v = getvalue(&vbuf, &t, 1);
+	DPUTS(!v, "BUG: value not found for new parameter");
     }
     setnumvalue(v, val);
     unqueue_signals();
diff -u zsh-forge/current/Src/subst.c zsh-4.0/Src/subst.c
--- zsh-forge/current/Src/subst.c	Fri May 18 09:15:52 2001
+++ zsh-4.0/Src/subst.c	Fri May 18 13:34:34 2001
@@ -1149,6 +1149,7 @@
 		isarr = 0;
 	    }
 	    pm = createparam(nulstring, isarr ? PM_ARRAY : PM_SCALAR);
+	    DPUTS(!pm, "BUG: parameter not created");
 	    if (isarr)
 		pm->u.arr = aval;
 	    else

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: PATCH: Re: zsh4.0.1-pre4 : backticks fail if SHLVL nonexistant

[Peter Stephenson]

"Bart Schaefer" wrote:
> There's some stuff going on here with `outputradix' that I'm not sure is
> correct.  setiparam() was ignoring the outputradix, setnparam() was using
> it; maybe that's because only setnparam() gets called from the math code?

Yes, setiparam() doesn't need it.

On the other problem:

> Richard Curnow wrote:
> > zshparam.1 says that setting manpath will change MANPATH and vice versa.
> > If I change MANPATH, I don't see manpath getting changed.
> 
> You haven't unset that in your .zshenv too, have you?  In that case it will
> (temporarily) lose its special connection to $manpath.  This seems to be a
> bug, because it happens even if $manpath is never unset.

I'm no longer convinced this is even a bug, if this is really what the
problem is.  The code quite explicitly unsets both of such a pair --- the
connection isn't lost, but manpath remains unset when you set MANPATH, so
you have to set both.  For user-tied variables with `typeset -T', the
documentation is clear about this, though it isn't explicit for pre-defined
path variables.  It's tricky to have them only unsetting one of a pair,
which causes a lot of grief with the internal state.  Maybe the answer is
to make sure `createparam' is called for the other value if it is also
unset?

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@csr.com
Web: http://www.pwstephenson.fsnet.co.uk

PATCH: unset path problem

[Peter Stephenson]

Peter Stephenson wrote:
> > Richard Curnow wrote:
> > > If I change MANPATH, I don't see manpath getting changed.
> > 
> > You haven't unset that in your .zshenv too, have you?
> 
> Maybe the answer is to make sure `createparam' is called for the other
> value if it is also unset?

It does seem to be this simple; it's logical that if they both get unset at
once, but retain their specialness, then they should both be recreated at
once.  I've added a couple of tests for this and the bug fixed by Bart, and
some documentation for special tied variables --- was there really none
before, or have I missed it?

Index: Src/params.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/params.c,v
retrieving revision 1.45
diff -u -r1.45 params.c
--- Src/params.c	2001/05/19 00:31:23	1.45
+++ Src/params.c	2001/05/19 23:36:50
@@ -669,6 +669,12 @@
 	if (oldpm && (oldpm->level == locallevel || !(flags & PM_LOCAL))) {
 	    if (!(oldpm->flags & PM_UNSET) || (oldpm->flags & PM_SPECIAL)) {
 		oldpm->flags &= ~PM_UNSET;
+		if ((oldpm->flags & PM_SPECIAL) && oldpm->ename) {
+		    Param altpm = 
+			(Param) paramtab->getnode(paramtab, oldpm->ename);
+		    if (altpm)
+			altpm->flags &= ~PM_UNSET;
+		}
 		return NULL;
 	    }
 	    if ((oldpm->flags & PM_RESTRICTED) && isset(RESTRICTED)) {
Index: Test/D04parameter.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/D04parameter.ztst,v
retrieving revision 1.1
diff -u -r1.1 D04parameter.ztst
--- Test/D04parameter.ztst	2001/04/02 12:34:06	1.1
+++ Test/D04parameter.ztst	2001/05/19 23:36:51
@@ -526,3 +526,23 @@
 >fixed
 >the
 >shell
+
+  unset SHLVL
+  (( SHLVL++ ))
+  print $SHLVL
+0:Unsetting and recreation of numerical special parameters
+>1
+
+  unset manpath
+  print $+MANPATH
+  manpath=(/here /there)
+  print $MANPATH
+  unset MANPATH
+  print $+manpath
+  MANPATH=/elsewhere:/somewhere
+  print $manpath
+0:Unsetting and recreation of tied special parameters
+>0
+>/here:/there
+>0
+>/elsewhere /somewhere
Index: Doc/Zsh/params.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/params.yo,v
retrieving revision 1.7
diff -u -r1.7 params.yo
--- Doc/Zsh/params.yo	2001/04/22 21:02:33	1.7
+++ Doc/Zsh/params.yo	2001/05/19 23:36:52
@@ -632,7 +632,17 @@
 enditem()
 texinode(Parameters Used By The Shell)()(Parameters Set By The Shell)(Parameters)
 sect(Parameters Used By The Shell)
-The following parameters are used by the shell:
+The following parameters are used by the shell.
+
+In cases where there are two parameters with an upper- and lowercase
+form of the same name, such as tt(path) and tt(PATH), the lowercase form
+is an array and the uppercase form is a scalar with the elements of the
+array joined together by colons.  These are similar to tied parameters
+created via `tt(typeset -T)'.  The normal use for the colon-separated
+form is for exporting to the environment, while the array form is easier
+to manipulate within the shell.  Note that unsetting either of the pair
+will unset the other; they retain their special properties when
+recreated, and recreating one of the pair will recreate the other.
 
 startitem()
 vindex(ARGV0)

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@csr.com
Web: http://www.pwstephenson.fsnet.co.uk