That infinite loop in Geoff's second zle_refresh patch

That infinite loop in Geoff's second zle_refresh patch

[Bart Schaefer]

I don't know if this is the same one that Zoltan saw, because when I
got it, it had nothing to do with the history ... the loop happens in
refreshline() when computing whether it's cheaper to insert or delete
a character than it is to reprint.  The first time around the loop,
we call tcdelcost(0) which is always less than any nonzero pfxlen(),
so we delete no characters and then go around the outer loop again,
forever.

The optimization is only attempted if there's no RPS1, which may be
why I couldn't reproduce it when Zoltan first reported it.

The patch below changes starting count from 0 to 1, which stops the
infinite loop, but feels wrong to me.  I can't see any reason to call
tcdelcost(0), but that code was there before either of Geoff's recent
patches; the only nearby difference since pre2 is a change from

	if ((ln || !put_rpmpt || !oput_rpmpt)) {
	    if (tccan(TCDEL) && nl[1] && ol[1] && (ol[1] != nl[1])) {
		/* ... */
	    }
	    if ((vln != lines - 1) &&	/* not on last line */
		tccan(TCINS) && nl[1] && ol[1] && (ol[1] != nl[1])) {
		/* ... */
	    }
	}
To
	if ((ln || !put_rpmpt || !oput_rpmpt) 
	    && (nl[1] && ol[1] && nl[1] != ol[1])) { 
	    if (tccan(TCDEL)) {
		/* ... */
	    }
	    if ((vln != lines - 1) && tccan(TCINS)) {	/* not on last line */
		/* ... */
	    }
	}

which sure looks harmless to me.

I'd think that rather than counting up from zero calling tcdelcost(),
we'd want to start with the largest number of deletable/insertable
characters that we can, then count *down* until tcdelcost(i) < pfxlen().
But maybe I'm just not following this correctly ...

Anyway, here's the patch to prevent the infinite loop.  Geoff, please
throw rocks if this is going to screw up for some other reason.

*** zle_refresh.c.00	Tue Jul 16 21:34:05 1996
--- zle_refresh.c	Tue Jul 16 21:34:54 1996
***************
*** 615,621 ****
  	   eg. oldline: hifoobar } hopefully cheaper here to delete two
  	       newline: foobar	 } characters, then we have six matches */
  	    if (tccan(TCDEL)) {
! 		for (i = 0, p1 = ol; *p1; p1++, i++)
  		    if (tcdelcost(i) < pfxlen(p1, nl)) {
  			tc_delchars(i);
  			SELECT_ADD_COST(i);
--- 615,621 ----
  	   eg. oldline: hifoobar } hopefully cheaper here to delete two
  	       newline: foobar	 } characters, then we have six matches */
  	    if (tccan(TCDEL)) {
! 		for (i = 1, p1 = ol; *p1; p1++, i++)
  		    if (tcdelcost(i) < pfxlen(p1, nl)) {
  			tc_delchars(i);
  			SELECT_ADD_COST(i);
***************
*** 631,637 ****
  	   undesired scrolling occurs due to `illegal' characters on screen */
  
  	    if (tccan(TCINS) && (vln != lines - 1)) {	/* not on last line */
! 		for (i = 0, p1 = nl; *p1; p1++, i++)
  		    if (tcinscost(i) < pfxlen(p1, ol)) {
  			tc_inschars(i);
  			SELECT_ADD_COST(2 * i);
--- 631,637 ----
  	   undesired scrolling occurs due to `illegal' characters on screen */
  
  	    if (tccan(TCINS) && (vln != lines - 1)) {	/* not on last line */
! 		for (i = 1, p1 = nl; *p1; p1++, i++)
  		    if (tcinscost(i) < pfxlen(p1, ol)) {
  			tc_inschars(i);
  			SELECT_ADD_COST(2 * i);

Re: That infinite loop in Geoff's second zle_refresh patch

[Peter Stephenson]

schaefer@candle.brasslantern.com wrote:
> The patch below changes starting count from 0 to 1, which stops the
> infinite loop, but feels wrong to me.  I can't see any reason to call
> tcdelcost(0), but that code was there before either of Geoff's recent
> patches; the only nearby difference since pre2 is a change from

This has a bad effect:  inserting or deleting on a continued line
before the end of the line redisplays the rest of line wrongly.  I
haven't looked into the details.  (Maybe I got confused; my
zle_refresh.c is different from the one in the patch, so I'm either
more or less up to date.)

-- 
Peter Stephenson <pws@ifh.de>       Tel: +49 33762 77366
WWW:  http://www.ifh.de/~pws/       Fax: +49 33762 77330
Deutches Electronen-Synchrotron --- Institut fuer Hochenergiephysik Zeuthen
DESY-IfH, 15735 Zeuthen, Germany.

Re: That infinite loop in Geoff's second zle_refresh patch

[Geoff Wing]

pws@ifh.de wrote:
:schaefer@candle.brasslantern.com wrote:
:> The patch below changes starting count from 0 to 1, which stops the
:> infinite loop, but feels wrong to me.  I can't see any reason to call
:> tcdelcost(0), but that code was there before either of Geoff's recent
:> patches; the only nearby difference since pre2 is a change from

Looks fine to me.  I give it my blessing :-)  
The "tcdelcost(i) < pfxlen(p1, nl)" stuff is only a rough estimate anyway.
You'd need to add costs of moving right and other stuff which is more
complex than justifies the reward (eg. you might save one or two chars 
output - and there are other areas that could use one or two character
cleanup first).

:This has a bad effect:  inserting or deleting on a continued line
:before the end of the line redisplays the rest of line wrongly.  I
:haven't looked into the details.  (Maybe I got confused; my
:zle_refresh.c is different from the one in the patch, so I'm either
:more or less up to date.)

Um, what stuffs up?  Is this on an xterm?  An example?
I fixed up some of the continued line stuff so cut/paste works better on
xterms and similar 'cos I seem to remember you were one of those asking
for it(?)
-- 
Mason [G.C.W]  mason@werple.mira.net.au    "Hurt...Agony...Pain...LOVE-IT"

Re: That infinite loop in Geoff's second zle_refresh patch

[Bart Schaefer]

On Jul 17, 12:03pm, Peter Stephenson wrote:
> Subject: Re: That infinite loop in Geoff's second zle_refresh patch
> schaefer@candle.brasslantern.com wrote:
> > The patch below changes starting count from 0 to 1, which stops the
> > infinite loop, but feels wrong to me.
> 
> This has a bad effect:  inserting or deleting on a continued line
> before the end of the line redisplays the rest of line wrongly.

My patch depended on Goeff's second zle_refresh.c patch, which was not
included in pre3.  Could it be that you don't have Geoff's patch?
Should I send a complete diff of my zle_refresh.c against pre3?  (I
have other patches from Zefram in there, too.)

I still don't understand (a) why the code worked before, as it was
calling tcdelcost(0) long ago; nor (b) what made it break this time.
Any insight, Geoff?

On Jul 18, 12:21am, Geoff Wing wrote:
> Subject: Re: That infinite loop in Geoff's second zle_refresh patch
> 
> Looks fine to me.  I give it my blessing :-)  

And there's no problem with the fact that `for (...; *p1; p1++, i++)'
now has i indexing one position farther into p1?  (That looked OK to
me -- in fact, it made more sense than what was there before, as i
is now the length of the substring that gets printed out of p1 -- but
I don't deeply understand the surrounding code, so ....)

> :This has a bad effect:  inserting or deleting on a continued line
> :before the end of the line redisplays the rest of line wrongly.
> 
> Um, what stuffs up?  Is this on an xterm?  An example?

Yes, an example would be helpful.

Re: That infinite loop in Geoff's second zle_refresh patch

[Peter Stephenson]

schaefer@candle.brasslantern.com wrote:
> On Jul 17, 12:03pm, Peter Stephenson wrote:
> > Subject: Re: That infinite loop in Geoff's second zle_refresh patch
> > schaefer@candle.brasslantern.com wrote:
> > > The patch below changes starting count from 0 to 1, which stops the
> > > infinite loop, but feels wrong to me.
> > 
> > This has a bad effect:  inserting or deleting on a continued line
> > before the end of the line redisplays the rest of line wrongly.
> 
> My patch depended on Goeff's second zle_refresh.c patch, which was not
> included in pre3.  Could it be that you don't have Geoff's patch?

This seems to have been the problem.  Bart sent me the complete thing
and everything seems to be OK.  False alarm.

-- 
Peter Stephenson <pws@ifh.de>       Tel: +49 33762 77366
WWW:  http://www.ifh.de/~pws/       Fax: +49 33762 77330
Deutches Electronen-Synchrotron --- Institut fuer Hochenergiephysik Zeuthen
DESY-IfH, 15735 Zeuthen, Germany.

3rd zle_refresh.c patch (Was: That infinite loop in Geoff's second zle_refresh patch)

[Geoff Wing]

Bart Schaefer wrote:
:On Jul 17, 12:03pm, Peter Stephenson wrote:
:> schaefer@candle.brasslantern.com wrote:
:> > The patch below changes starting count from 0 to 1, which stops the
:> > infinite loop, but feels wrong to me.
:> This has a bad effect:  inserting or deleting on a continued line
:> before the end of the line redisplays the rest of line wrongly.
:I still don't understand (a) why the code worked before, as it was
:calling tcdelcost(0) long ago; nor (b) what made it break this time.
:Any insight, Geoff?

Mysterious are the ways of the C compilers.

:On Jul 18, 12:21am, Geoff Wing wrote:
:> Looks fine to me.  I give it my blessing :-)  

Whoops, just a bit fast about that.  I hurriedly wrote this because I
was about to leave work, then thought of a problem driving home from work
(OK, OK, it was past midnight, forgive me please :-)

:And there's no problem with the fact that `for (...; *p1; p1++, i++)'
:now has i indexing one position farther into p1?  (That looked OK to
:me -- in fact, it made more sense than what was there before, as i
:is now the length of the substring that gets printed out of p1 -- but
:I don't deeply understand the surrounding code, so ....)

Um, yes, it's a problem. i  is wrong.  For delete stuff - if we match two
chars ahead, then we've incremented p1 twice and i should equal 2. (Well,
that's my logic anyway).  Similarly for insert stuff.
However, given there's implicitly never a match on the first char anyway,
we can start with  i = 1.

This is a patch on my previous two patches (not including Bart Schaefer's
patch in 1674, nor zefram's patch in 1678)  (This better be right, it's 
4 am and I'm going to go to sleep!!)


*** zle_refresh.c.~2~	Mon Jul 15 22:05:16 1996
--- zle_refresh.c	Thu Jul 18 03:59:23 1996
***************
*** 620,626 ****
  	   eg. oldline: hifoobar } hopefully cheaper here to delete two
  	       newline: foobar	 } characters, then we have six matches */
  	    if (tccan(TCDEL)) {
! 		for (i = 0, p1 = ol; *p1; p1++, i++)
  		    if (tcdelcost(i) < pfxlen(p1, nl)) {
  			tc_delchars(i);
  			SELECT_ADD_COST(i);
--- 620,626 ----
  	   eg. oldline: hifoobar } hopefully cheaper here to delete two
  	       newline: foobar	 } characters, then we have six matches */
  	    if (tccan(TCDEL)) {
! 		for (i = 1, p1 = ol + 1; *p1; p1++, i++)
  		    if (tcdelcost(i) < pfxlen(p1, nl)) {
  			tc_delchars(i);
  			SELECT_ADD_COST(i);
***************
*** 636,642 ****
  	   undesired scrolling occurs due to `illegal' characters on screen */
  
  	    if (tccan(TCINS) && (vln != lines - 1)) {	/* not on last line */
! 		for (i = 0, p1 = nl; *p1; p1++, i++)
  		    if (tcinscost(i) < pfxlen(p1, ol)) {
  			tc_inschars(i);
  			SELECT_ADD_COST(2 * i);
--- 636,642 ----
  	   undesired scrolling occurs due to `illegal' characters on screen */
  
  	    if (tccan(TCINS) && (vln != lines - 1)) {	/* not on last line */
! 		for (i = 1, p1 = nl + 1; *p1; p1++, i++)
  		    if (tcinscost(i) < pfxlen(p1, ol)) {
  			tc_inschars(i);
  			SELECT_ADD_COST(2 * i);






-- 
Mason [G.C.W]  mason@werple.mira.net.au    "Hurt...Agony...Pain...LOVE-IT"