svn commit: r197752 - head/lib/libc/stdio

[David Schultz]

On Mon, Jan 11, 2010, Max Laier wrote:
> On Sunday 04 October 2009 21:43:36 David Schultz wrote:
> > Author: das
> > Date: Sun Oct  4 19:43:36 2009
> > New Revision: 197752
> > URL: http://svn.freebsd.org/changeset/base/197752
> > 
> > Log:
> >   Better glibc compatibility for getline/getdelim:
> > 
> >   - Tolerate applications that pass a NULL pointer for the buffer and
> >     claim that the capacity of the buffer is nonzero.
> > 
> >   - If an application passes in a non-NULL buffer pointer and claims the
> >     buffer has zero capacity, we should free (well, realloc) it
> >     anyway. It could have been obtained from malloc(0), so failing to
> >     free it would be a small memory leak.
> > 
> >   MFC After:	2 weeks
> >   Reported by:	naddy
> >   PR:		ports/138320
> > 
> > Modified:
> >   head/lib/libc/stdio/getdelim.c
> > 
> > Modified: head/lib/libc/stdio/getdelim.c
> > ===========================================================================
> > === --- head/lib/libc/stdio/getdelim.c	Sun Oct  4 19:03:32 2009	(r197751)
> >  +++ head/lib/libc/stdio/getdelim.c	Sun Oct  4 19:43:36 2009	(r197752) @@
> >  -120,8 +120,8 @@ getdelim(char ** __restrict linep, size_
> >  		goto error;
> >  	}
> > 
> > -	if (*linecapp == 0)
> > -		*linep = NULL;
> > +	if (*linep == NULL)
> > +		*linecapp = 0;
> > 
> >  	if (fp->_r <= 0 && __srefill(fp)) {
> >  		/* If fp is at EOF already, we just need space for the NUL. */
> 
> I think we should have kept the original if case here, as well.  Otherwise 
> something like this might fail:
> 
> 	char *line;				/* note uninitialized */
> 	size_t len = 0;
> 	getline(&line, &len, fd);
> 
> and I think it is a reasonable thing to pass in an uninitialized pointer if 
> you tell that there is no space associated with it, yet.
> 
> I don't know if there are many (ab)uses of getline like this, but I was just 
> bitten by it.

The trouble is that in many malloc() implementations, including
ours, malloc(0) actually makes a small allocation instead of
returning NULL.  Hence, with your proposed change, code such as
the following would result in a memory leak:

       size_t len = 0;
       char *line = malloc(len);
       getline(&line, &len, fd);

POSIX says that *linep has to be a valid argument to the free(),
but it's allowed to point to a buffer that's larger than
advertised (e.g., a 128-byte buffer when *linecapp is 0.)  It's a
bad API; what's missing is a "line buffer" ADT that hides the
details of its implementation.  But it's still a bit better than
fgetln() and way better than something hand-rolled.