suggested PATCH

suggested PATCH

[Sven Wischnowsky]

I'm pretty sure I found a bug, I'm just not sure if the result was
intended (and I don't like it):

If you exit a shell with running jobs you get this `n jobs SIGHUPed'
message. Exiting is done in zexit(), which calls killrunjobs(), which
in turn prints that message using zerr() -- and that sets errflag=1.
Later, zexit() calls savehistfile(), that calls getiparam("SAVEHIST")
to get the number of lines that should be saved. That in turn uses
(vie getinvalue()) mathevali() to turn the parameter value into an
integer. And matheval() calls mathevall() which calls mathparse()
which -- now it comes! -- returns immediatly when errflag is set. This 
causes mathevall() to return some random value. In my case it was -4
and that meant that the history wasn't saved.

And I don't like having my history not saved just because the error
message about SIGHUPed jobs was printed.

So this patch makes the returned value in mathevall() be initialised
to zero, but it also makes errflag be reset in zexit(). It's that last 
part I'm not so sure about -- maybe you all like this history-saving-
avoidance?

Bye
 Sven

diff -u oldsrc/builtin.c Src/builtin.c
--- oldsrc/builtin.c	Mon Nov  8 11:23:25 1999
+++ Src/builtin.c	Wed Nov 10 09:02:32 1999
@@ -3209,9 +3209,11 @@
 	if (in_exit++ && from_signal) {
 	    LASTALLOC_RETURN;
 	}
-	if (isset(MONITOR))
+	if (isset(MONITOR)) {
 	    /* send SIGHUP to any jobs left running  */
 	    killrunjobs(from_signal);
+	    errflag = 0;
+	}
 	if (isset(RCS) && interact) {
 	    if (!nohistsave)
 		savehistfile(NULL, 1, HFILE_USE_OPTIONS);
diff -u oldsrc/math.c Src/math.c
--- oldsrc/math.c	Mon Nov  8 11:23:26 1999
+++ Src/math.c	Wed Nov 10 09:06:44 1999
@@ -876,6 +876,8 @@
     ptr = s;
     sp = -1;
     unary = 1;
+    stack[0].val.type = MN_INTEGER;
+    stack[0].val.u.l = 0;
     mathparse(prek);
     *ep = ptr;
     DPUTS(!errflag && sp,

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: suggested PATCH

[Bart Schaefer]

On Nov 10,  9:19am, Sven Wischnowsky wrote:
} Subject: suggested PATCH
}
} If you exit a shell with running jobs you get this `n jobs SIGHUPed'
} message. Exiting is done in zexit(), which calls killrunjobs(), which
} in turn prints that message using zerr() -- and that sets errflag=1.

Seems to me that ought to be calling zwarn().

} So this patch makes the returned value in mathevall() be initialised
} to zero

That's good.

} but it also makes errflag be reset in zexit(). It's that last 
} part I'm not so sure about -- maybe you all like this history-saving-
} avoidance?

No, that's got to be wrong.  Since killrunjobs() is never called from
anywhere except zexit(), I think using zwarn() rather than setting and
then clearing errflag is the way to go.

So, my suggestion is to use only the math.c hunk of Sven's patch, and
use this instead of the builtin.c hunk:

Index: Src/signals.c
===================================================================
@@ -576,7 +576,7 @@
                 killed++;
         }
     if (killed)
-        zerr("warning: %d jobs SIGHUPed", NULL, killed);
+        zwarn("warning: %d jobs SIGHUPed", NULL, killed);
 }
 
 

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