Date: Mon, 11 Jan 2010 19:40:07 -0500 From: David Schultz <das@freebsd.org> To: Max Laier <max@love2party.net> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r197752 - head/lib/libc/stdio Message-ID: <20100112004007.GA87632@zim.MIT.EDU> In-Reply-To: <201001110316.29816.max@love2party.net> References: <200910041943.n94JhaDg083487@svn.freebsd.org> <201001110316.29816.max@love2party.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100112004007.GA87632>