Re: Weirdness in comm(1)

[Roy Marples <roy%NetBSD.org@localhost>]

On Sunday 29 November 2009 01:43:30 Greg A. Woods wrote:
> At Sun, 29 Nov 2009 01:16:16 +0000, Roy Marples <roy%marples.name@localhost> 
> wrote:
> Subject: Re: Weirdness in comm(1)
>
> > On Sunday 29 November 2009 01:01:55 Greg A. Woods wrote:
> > > I think whomever munged fgetstr(3) to be based on getdelim(3) in
> > > -current may have broken it BADLY (likely, actually; re-introducing
> > > bogus line length limits, sigh) -- they also forgot to update the
> > > manual page (for both a new error and for its references).
> >
> > I'll update the manual page to reflect the new error tomorrow evening.
> > There is a good reason why the line length is now limited - the sudio
> > buffer length it uses is an int. Which is a little small for a size_t
> > that fgetln(3) wants to use.
>
> That's not a good reason _at_ _all_.
>
> The previous implementation correctly handled lines as long as it could
> realloc() a buffer to fit them; _and_ its documentation also allows for
> a standard error for the case where the allocation fails.

When I first looked at fgetstr.c and it's realloction, my first reaction 
was that it would crash or have undefined behaviour when newsize > 
INT_MAX. Closer inspection shows that it just reallocs the buffer on 
each call when newsize > INT_MAX. In other words, it works, just very 
inefficiently.

Attached is a patch that should restore this behaviour. Untested though.
Does this appease you to any extent?

Thanks

Roy

Index: fgetstr.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdio/fgetstr.c,v
retrieving revision 1.8
diff -u -p -r1.8 fgetstr.c
--- fgetstr.c   15 Oct 2009 00:36:24 -0000      1.8
+++ fgetstr.c   29 Nov 2009 09:05:35 -0000
@@ -59,16 +59,8 @@ __fgetstr(FILE *__restrict fp, size_t *_
        size = fp->_lb._size;
        *lenp = __getdelim(&p, &size, sep, fp);
        fp->_lb._base = (unsigned char *)p;
-       /* The struct size variable is only an int ..... */
-       if (size > INT_MAX) {
-               fp->_lb._size = INT_MAX;
-               errno = EOVERFLOW;
-               goto error;
-       }
-       fp->_lb._size = (int)size;
-       if (*lenp != 0 && *lenp < SIZE_MAX - 1)
-               return p;
-error:
-       *lenp = 0;
-       return NULL;
+       fp->_lb._size = size;
+       if (*lenp == 0 || *lenp == SIZE_MAX)
+               return NULL;
+       return p;
 }