Re: Bug(?) in builtin r

[Peter Stephenson <pws@csr.com>]

On Thu, 24 Jul 2008 16:52:39 +0200
"Richard Hartmann" <richih.mailinglist@gmail.com> wrote:
> Hi all,
> 
> I am sure most of you are aware of this, but it has always
> ailed me and should probably be changed.
> If you have 100 commands on your stack and call
> 
>   r 101
> 
> it will recurse endlessly or as long as the box allows it to
> do until killing it. Unless there are useful aspects to this
> behaviour, it is basically a nice way to kill your shell.

Hmm... repeating the current line and doing something else with it (rather
than just repeating it) is potentially useful.  However, in zsh this is
handled recursively, so you quickly get into problems on the stack.  (I
have a suspicion in ksh it puts it onto the input stack and this problem is
avoided.)  It would therefore be safer to disallow it.

This disallows the specific case that the range includes the current
history entry and nothing before it, and there's no editor.  Otherwise it
either truncates the history so that the current line isn't included, or if
you've specified an editor it lets you edit the current line, too, if you
really want.  It's not clear that last bit is actually useful and it might
be better consistently to ignore the current history line here.

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.197
diff -u -r1.197 builtin.c
--- Src/builtin.c	17 Jul 2008 11:27:57 -0000	1.197
+++ Src/builtin.c	24 Jul 2008 17:13:39 -0000
@@ -1446,20 +1446,28 @@
 	    unqueue_signals();
 	    zwarnnam("fc", "can't open temp file: %e", errno);
 	} else {
+	    char *editor;
+
+	    if (func == BIN_R)
+		editor = "-";
+	    else if (OPT_HASARG(ops, 'e'))
+		editor = OPT_ARG(ops, 'e');
+	    else
+		editor = getsparam("FCEDIT");
+	    if (!editor)
+		editor = getsparam("EDITOR");
+	    if (!editor)
+		editor = DEFAULT_FCEDIT;
+	    if (last >= curhist && !strcmp(editor, "-")) {
+		last = curhist - 1;
+		if (first > last) {
+		    unqueue_signals();
+		    zwarnnam("fc", "invalid use of current history line");
+		    return 1;
+		}
+	    }
 	    ops->ind['n'] = 1;	/* No line numbers here. */
 	    if (!fclist(out, ops, first, last, asgf, pprog)) {
-		char *editor;
-
-		if (func == BIN_R)
-		    editor = "-";
-		else if (OPT_HASARG(ops, 'e'))
-		    editor = OPT_ARG(ops, 'e');
-		else
-		    editor = getsparam("FCEDIT");
-		if (!editor)
-		    editor = getsparam("EDITOR");
-		if (!editor)
-		    editor = DEFAULT_FCEDIT;
 
 		unqueue_signals();
 		if (fcedit(editor, fil)) {

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070