[PATCH RFA] libgloss read.c ... totally broken!

[cgd at broadcom dot com]

So, I was truly shocked to discover... libgloss/read.c is totally
broken.  it seems to have been _always_ been broken in the public CVS,
at least w.r.t. common sense and the read() semantics expected by the
newlib stdio implementation.

I'm kinda surprised that nobody's noticed this problem before, I guess
most people use libgloss w/ targets that don't use read.c, or just
don't use input at all!


Two big problems, which this patch fixes:

(1) can write outside the buffer provided by the caller!

    the caller-provided buffer goes from buf[0] .. buf[nbytes-1].

    consider the 'for' loop when i == (nbytes-1), and the read
    character is \n or \r:

	buf[nbytes-1] = the read character;
	buf[nbytes-1+1] = \0;

    the latter is past the end of the user-supplied buffer!  (Also,
    it's pointless to NUL-terminate the buffer.  8-)

(2) on empty input line, read returns 0!

    if first character returned by 'inbyte' is \n or \r.  the break
    inside the loop is hit, but 'i' is never incremented.  0 is
    returned.

    the stdio code (refill.c, ca. line 103) assumes that read
    returning 0 means EOF, because, well, that's what normal UNIX
    blocking read returning 0 means.  The Wrong Thing results.


The solution is:

* don't NUL-terminate the buffer,

* when you get CR or NL, make sure it's reflected in the return value
  (by incrementing 'i' before the break).


So, more generally about libgloss read(), is there _any_ point in
having it do this feeble attempt at line buffering?  Why not just
return one character at a time?  (for apps which read lines from
stdio, it'll work just fine...)

Then, once the existing buffering is squished: that would be a 'raw'
read function, it might also be nice to have a 'cooked' read function
that let people backspace, etc. while entering the line.  (The UNIX
way to select between them is, of course, ioctl, but I'd be quite
happy to do it for libgloss w/ a special function call that the user
program could make if they wanted the cooked behaviour...)

(yes, that's right, somehow i've found people who want to use
newlib+libgloss with simple, interactive programs on real raw
hardware...  8-)


chris
===================================================================
2002-07-17  Chris Demetriou  <cgd@broadcom.com>

	* read.c (read): Don't assign past end of buffer, fix return value.

Index: read.c
===================================================================
RCS file: /cvs/src/src/libgloss/read.c,v
retrieving revision 1.1
diff -u -p -r1.1 read.c
--- read.c	17 Mar 2000 22:48:49 -0000	1.1
+++ read.c	18 Jul 2002 05:58:27 -0000
@@ -31,7 +31,7 @@ _DEFUN (read, (fd, buf, nbytes),
   for (i = 0; i < nbytes; i++) {
     *(buf + i) = inbyte();
     if ((*(buf + i) == '\n') || (*(buf + i) == '\r')) {
-      (*(buf + i + 1)) = 0;
+      i++;
       break;
     }
   }