(Fwd) Re: input.c and here documents bugfixes

[schaefer@xxxxxxxxxx (Barton E. Schaefer)]

On Sep 22,  7:03pm, Zoltan Hidvegi wrote:
} Subject: Re: input.c and here documents bugfixes
}
} !     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].
} 
} !     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.
} 
} 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 ingets() never called with this value).

OK, I'm convinced.  I still think ingets() should handle n <= 1, though;
just because it's not called that way now doesn't mean somebody else won't
decide to call it that way in the future.

Attached are Zoltan's two messages that didn't go to zsh-workers.  Here's
a slight variant of Zoltan's patch to handle n <= 1.

--- input.c.0	Thu Sep 21 09:33:45 1995
+++ input.c	Fri Sep 22 11:29:23 1995
@@ -272,10 +272,12 @@
 ingets(char *buf, int n)
 {
     int l;
-    for (l = 0; l < n - 1; l++)
-      if ((buf[l] = ingetc()) == '\n' || lexstop)
-          break;
-    buf[l + (lexstop ? 0 : 1)] = 0;
+
+    for (l = 0; l < n - 1 && (buf[l++] = ingetc()) != '\n' && !lexstop;)
+	;
+    if (lexstop && l > 0)
+	--l;
+    buf[l] = 0;
 
     return (!lexstop || l) ? buf : NULL;
 }

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

--- Begin Message ---

• From: hzoli@xxxxxxxxxx (Zoltan Hidvegi)
• To: schaefer@xxxxxxxxxx
• Subject: Re: input.c and here documents bugfixes
• Date: Fri, 22 Sep 1995 18:09:11 +0200 (MET DST)
• In-reply-to: <950921110903.ZM9297@xxxxxxxxxxxxxxxxx> from "Barton E. Schaefer" at Sep 21, 95 11:09:02 am

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;
  }

--- End Message ---

--- Begin Message ---

• From: hzoli@xxxxxxxxxx (Zoltan Hidvegi)
• To: schaefer@xxxxxxxxxx
• Subject: Re: input.c and here documents bugfixes
• Date: Fri, 22 Sep 1995 19:03:32 +0200 (MET DST)
• In-reply-to: <9509220938.ZM4578@xxxxxxxxxxxxxxxx> from "Bart Schaefer" at Sep 22, 95 09:38:15 am

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

--- End Message ---