Re: Segfault in hist.c

[Peter Stephenson <Peter.Stephenson@xxxxxxx>]

On Mon, 11 Jul 2011 13:56:55 +0200
Frank Terbeck <ft@xxxxxxxxxxxxxxxxxxx> wrote:
> This just came up on IRC:
> 
> zsh% alias foo=bar
> zsh% foo\<RET>
> > <C-c>
> zsh% <RET>
>  hist.c:1165: BUG: chline is NULL in hend()
> [1] 8401 segmentation fault (core dumped)  PS1="zsh%# " zsh -f

Hmm.

I've tracked it down to a test in hbegin() when initialising the command
line after the aborted one.  At this point inbufflags hasn't been
initialised and it misfires by testing the "alias" flag still in effect
from the failed line and is tricked into thinking the history mechanism
shouldn't be used.

I can't see why that test could be useful, and taking it out didn't seem
to break anything.  It comes from:

revision 1.2
date: 2000/04/12 08:24:16;  author: wischnow;  state: Exp;  lines: +718 -291
new widget copy-prev-shell-word, like copy-prev-word but uses shell parsing to find word (10685)
----------------------------

so presumably is related to some grungy undocumented zle interaction.

About the only course of action I can see is to take it out and see if
something possibly in copy-prev-shell-word breaks; whatever it's for, it
can't possibly be the right way of doing it.  I tried that ZLE function
but didn't see anything odd.  My hope is that if it was ever needed it
was a nasty hack to cover up the fact that we didn't properly save and
restore as many things when calling into the main shell for input in
ZLE.

The zle hunk is just paranoia:  I don't like the idea of ZLE returning a
non-zero command line if there's been an error.  I think this just fixes
an inconsistency.

Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.112
diff -p -u -r1.112 hist.c
--- Src/hist.c	3 Jun 2011 19:54:44 -0000	1.112
+++ Src/hist.c	11 Jul 2011 13:38:07 -0000
@@ -876,7 +876,18 @@ hbegin(int dohist)
 	stophist = (!interact || unset(SHINSTDIN)) ? 2 : 0;
     else
 	stophist = 0;
-    if (stophist == 2 || (inbufflags & INP_ALIAS)) {
+    /*
+     * pws: We used to test for "|| (inbufflags & INP_ALIAS)"
+     * in this test, but at this point we don't have input
+     * set up up so this can trigger unnecessarily.
+     * I don't see how the test at this point could ever be
+     * useful, since we only get here when we're initialising
+     * the history mechanism, before we've done any input.
+     *
+     * (I also don't see any point where this function is called with
+     * dohist=0.)
+     */
+    if (stophist == 2) {
 	chline = hptr = NULL;
 	hlinesz = 0;
 	chwords = NULL;
Index: Src/Zle/zle_main.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_main.c,v
retrieving revision 1.128
diff -p -u -r1.128 zle_main.c
--- Src/Zle/zle_main.c	26 May 2011 09:19:11 -0000	1.128
+++ Src/Zle/zle_main.c	11 Jul 2011 13:38:07 -0000
@@ -1233,7 +1233,7 @@ zleread(char **lp, char **rp, int flags,
     alarm(0);
 
     freeundo();
-    if (eofsent) {
+    if (eofsent || errflag) {
 	s = NULL;
     } else {
 	zleline[zlell++] = ZWC('\n');

-- 
Peter Stephenson <pws@xxxxxxx>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog