Re: input.c and here documents bugfixes

Re: input.c and here documents bugfixes

[Bart Schaefer]

[ You sent your message only to me, Zoltan ... it didn't go to the list. ]

On Sep 22,  6:09pm, Zoltan Hidvegi wrote:
} Subject: Re: input.c and here documents bugfixes
}
} > I think Zoltan's input.c patch was incorrect
} > 
} -- patch deleted ---
} 
} Sorry, I forgot the semicolon at the end. The patch below should be correct.
} 
} The problem with the original file is that is can exit from the for loop on
} two conditions.  If it exits when the buffer is filled up, the l counter is
} one more than otherwise.

But that's the right thing to do, unless ingetc() has changed a lot from
hgetch().  Originally, hgetch() would return a space that wasn't really
in the input stream whenever lexstop was true; and hgets() [which became
ingets() recently] would have to trim that space off to return the real
input.  If the buffer is filled up, then the last character returned by
ingetc() is a real part of the stream, and the counter should be one
greater so that the '\0' string-terminator will be inserted after the
last character read.

Your loop is completely equivalent as far as I can tell, except that if
n <= 1 and lexstop happens to already be true before ingets() is called,
you'll write a '\0' into buf[-1].

I still think the original version is correct.  Can you give an example
where it fails and your version does something different?

-- 
Bart Schaefer                                      Vice President, Technology
schaefer@z-code.com                                Z-Code / NCD Software Inc.

What quantity of engineers is required to rotationally reconfigure an
electronically-actuated filament-enabled photon-wave generator?

Re: input.c and here documents bugfixes

[Zoltan Hidvegi]

Bart Schaefer wrote:
> } The problem with the original file is that is can exit from the for loop on
> } two conditions.  If it exits when the buffer is filled up, the l counter is
> } one more than otherwise.
> 
> But that's the right thing to do, unless ingetc() has changed a lot from
> hgetch().  Originally, hgetch() would return a space that wasn't really
> in the input stream whenever lexstop was true; and hgets() [which became
> ingets() recently] would have to trim that space off to return the real
> input.  If the buffer is filled up, then the last character returned by
> ingetc() is a real part of the stream, and the counter should be one
> greater so that the '\0' string-terminator will be inserted after the
> last character read.
> 
> Your loop is completely equivalent as far as I can tell, except that if
> n <= 1 and lexstop happens to already be true before ingets() is called,
> you'll write a '\0' into buf[-1].

Here is the original:

!     for (l = 0; l < n - 1; l++)
!       if ((buf[l] = ingetc()) == '\n' || lexstop)
!           break;
!     buf[l + (lexstop ? 0 : 1)] = 0;


If it exit on the l < n - 1 condition, buf[l-1] is the last character it
wrote, but in all other cases it is buf[l].  The (lexstop ? 0 : 1) hack does
what you said but if l = n - 1 (the buffer filled up) and lexstop is not yes
true (more to come) then buf[n] will be written, which is one byte over the
end of the buffer.  This means that it overwrites the next element on the
stack which may be a return address, a frame pointer or anything else.  And
here is the fixed version:

!     for (l = 0; l < n - 1 && (buf[l++] = ingetc()) != '\n' && !lexstop;);
!     if (lexstop)
!       l--;
!     buf[l] = '\0';

Note that here in all cases, buf[l-1] was the last character written.  This
means that in most cases the terminating null must be written to buf[l], but
if lexstop is one, I decrement l to delete the additional space returned by
ingetc().

The exit condition guarantees that l < n on exit from the loop, and l is not
incremented after the exit from the loop, so buf[l] is always in the usable
address range (unless n = 1 but htegs() never called with this value).

Zoltan

Re: input.c and here documents bugfixes

[Zoltan Hidvegi]

Barton E. Schaefer wrote:
> 
> I think Zoltan's input.c patch was incorrect.  The exec.c part of the
> patch is OK.  Here's the relevant bit:
> 
-- patch deleted ---

Sorry, I forgot the semicolon at the end. The patch below should be correct.

The problem with the original file is that is can exit from the for loop on
two conditions.  If it exits when the buffer is filled up, the l counter is
one more than otherwise.

Zoltan

*** 1.2	1995/09/19 17:57:52
--- input.c	1995/09/22 14:59:56
***************
*** 274,283 ****
  {
      int l;
  
!     for (l = 0; l < n - 1; l++)
! 	if ((buf[l] = ingetc()) == '\n' || lexstop)
! 	    break;
!     buf[l + (lexstop ? 0 : 1)] = 0;
  
      return (!lexstop || l) ? buf : NULL;
  }
--- 274,283 ----
  {
      int l;
  
!     for (l = 0; l < n - 1 && (buf[l++] = ingetc()) != '\n' && !lexstop;);
!     if (lexstop)
! 	l--;
!     buf[l] = '\0';
  
      return (!lexstop || l) ? buf : NULL;
  }

Re: input.c and here documents bugfixes

[Barton E. Schaefer]

I think Zoltan's input.c patch was incorrect.  The exec.c part of the
patch is OK.  Here's the relevant bit:

--- Forwarded mail from Zoltan Hidvegi  <hzoli@cs.elte.hu>

*** 1.2	1995/09/19 17:57:52
--- Src/input.c	1995/09/20 11:04:22
***************
*** 274,283 ****
  {
      int l;
  
!     for (l = 0; l < n - 1; l++)
! 	if ((buf[l] = ingetc()) == '\n' || lexstop)
! 	    break;
!     buf[l + (lexstop ? 0 : 1)] = 0;
  
      return (!lexstop || l) ? buf : NULL;
  }
--- 274,283 ----
  {
      int l;
  
!     for (l = 0; l < n - 1 && (buf[l++] = ingetc()) != '\n' && !lexstop;)
!     if (lexstop)
! 	l--;
!     buf[l] = '\0';
  
      return (!lexstop || l) ? buf : NULL;
  }


--- End of forwarded mail from Zoltan Hidvegi  <hzoli@cs.elte.hu>

I don't know whether that's supposed to be "for (... && !lexstop;);" or
whether the "if (lexstop)" is intended to be part of the loop (as it is
as it stands), but either way it's wrong -- if it's not part of the loop,
it can decrement `l' to less than zero when n <= 1, and if it *is* part
of the loop, it fails to decrement `l' at all.

Zoltan, why did you change this code in the first place?

-- 
Bart Schaefer                     Vice President, Technology, Z-Code Software
schaefer@z-code.com                  Division of NCD Software Corporation
http://www.well.com/www/barts

Re: input.c and here documents bugfixes

[Barton E. Schaefer]

On Sep 21,  1:19pm, Zoltan Hidvegi wrote:
} Subject: input.c and here documents bugfixes
}
} *** 1.2	1995/09/19 17:57:52
} --- Src/input.c	1995/09/20 11:04:22
} ***************
} --- 274,283 ----
}   {
}       int l;
}   
} !     for (l = 0; l < n - 1 && (buf[l++] = ingetc()) != '\n' && !lexstop;)
} !     if (lexstop)
} ! 	l--;
} !     buf[l] = '\0';
}   
}       return (!lexstop || l) ? buf : NULL;
}   }


Surely the semicolon is supposed to be OUTSIDE the right paren in that "for"?

-- 
Bart Schaefer                     Vice President, Technology, Z-Code Software
schaefer@z-code.com                  Division of NCD Software Corporation
http://www.well.com/www/barts

input.c and here documents bugfixes

[Zoltan Hidvegi]

A while ago I reported that here documents with lines longer that 255
characters cause SEGV on Linux.  Then I suspected that the bug is in input.c
(i.e. beta10 doesn't have this bug).  I was right, the patch can be seen
below.  However this doesn't fix all problems with here documents.  Zsh think
that the here-doc. is over when it sees the end mark after a
backslash/newline:

cat << EOF
foo\
EOF
EOF

should print fooEOF.  An other problem (after the input.c bug is fixed or if
vanilla beta10 is used), that if there is an end mark at the 256th position of
a line, zsh will think that the here document is over.  And an evem more
serious problem is that it discards TABs at the 256th position if <<- was
used.

All of these are fixed below.

Bye,
  Zoltan

*** 1.9	1995/09/21 10:30:39
--- Src/exec.c	1995/09/21 10:46:34
***************
*** 1703,1723 ****
  	if (!ingets(pbuf, sizeof(pbuf)))
  	    break;
  	bptr = pbuf;
! 	if (strip)
! 	    while (*bptr == '\t')
! 		bptr++;
! 	for (u = bptr, v = str; *u != '\n' && *v; u++, v++)
! 	    if (*u != *v)
  		break;
! 	if (!(*u == '\n' && !*v)) {
! 	    l = strlen(bptr);
! 	    if (!qt && l > 1 && bptr[l - 1] == '\n' && bptr[l - 2] == '\\')
! 		bptr[l -= 2] = '\0';
! 	    t = realloc(t, siz + l + 1);
! 	    strncpy(t + siz, bptr, l);
! 	    siz += l;
! 	} else
! 	    break;
      }
      t[siz] = '\0';
      if (siz && t[siz - 1] == '\n')
--- 1703,1724 ----
  	if (!ingets(pbuf, sizeof(pbuf)))
  	    break;
  	bptr = pbuf;
! 	if (!siz || t[siz-1] == '\n') {
! 	    if (strip)
! 		while (*bptr == '\t')
! 		    bptr++;
! 	    for (u = bptr, v = str; *u != '\n' && *v; u++, v++)
! 		if (*u != *v)
! 		    break;
! 	    if (*u == '\n' && !*v)
  		break;
! 	}
! 	l = strlen(bptr);
! 	if (!qt && l > 1 && bptr[l - 1] == '\n' && bptr[l - 2] == '\\')
! 	    bptr[l -= 2] = '\0';
! 	t = realloc(t, siz + l + 1);
! 	strncpy(t + siz, bptr, l);
! 	siz += l;
      }
      t[siz] = '\0';
      if (siz && t[siz - 1] == '\n')
*** 1.2	1995/09/19 17:57:52
--- Src/input.c	1995/09/20 11:04:22
***************
*** 274,283 ****
  {
      int l;
  
!     for (l = 0; l < n - 1; l++)
! 	if ((buf[l] = ingetc()) == '\n' || lexstop)
! 	    break;
!     buf[l + (lexstop ? 0 : 1)] = 0;
  
      return (!lexstop || l) ? buf : NULL;
  }
--- 274,283 ----
  {
      int l;
  
!     for (l = 0; l < n - 1 && (buf[l++] = ingetc()) != '\n' && !lexstop;)
!     if (lexstop)
! 	l--;
!     buf[l] = '\0';
  
      return (!lexstop || l) ? buf : NULL;
  }