[PATCH] libio: Add terminating NUL when the first character is EOF in getdelim [BZ #28038]

[Florian Weimer]

* Collin Funk:

> Hi Florian,
>
> Florian Weimer <fweimer@redhat.com> said:
>
>> * Collin Funk:
>> 
>> > POSIX requires that the buffer used by getdelim/getline add a
>> > terminating NUL whenever an EOF is read.
>> >
>> > * libio/iogetdelim.c (__getdelim): Add a NUL byte when the first
>> > __underflow is called.
>> > * libio/tst-getdelim.c (do_test): Add a test case for the bug.
>> >
>> > -- 8< --
>> >
>> > If this patch is okay for glibc, then I will push it to Gnulib as well
>> > since the function is mostly copied over there.
>> >
>> > ---
>> >  libio/iogetdelim.c   |  1 +
>> >  libio/tst-getdelim.c | 17 +++++++++++++++++
>> >  2 files changed, 18 insertions(+)
>> >
>> > diff --git a/libio/iogetdelim.c b/libio/iogetdelim.c
>> > index 0bfaef227a..1d89757352 100644
>> > --- a/libio/iogetdelim.c
>> > +++ b/libio/iogetdelim.c
>> > @@ -77,6 +77,7 @@ __getdelim (char **lineptr, size_t *n, int delimiter, FILE *fp)
>> >        if (__underflow (fp) == EOF)
>> >  	{
>> >  	  result = -1;
>> > +	  (*lineptr)[0] = '\0';
>> >  	  goto unlock_return;
>> >  	}
>> >        len = fp->_IO_read_end - fp->_IO_read_ptr;
>> 
>> Looks okay.  The buffer is already allocated at this point.  I don't
>> think there is backwards compatibility impact.
>
> I had thought this was harmless as well, but now am considering
> reverting it or introducing a compat symbol.
>
> There is more discussion in the bug report and on bug-gnulib [1][2]. The
> shortened story is that nbdkit used a loop like the following to get the
> last line of 'du -cs':
>
>     while (getline (&line, &len, fp) != -1)
>       ;
>     /* Process last line stored in LINE.  */
>
> This code was buggy before my patch in the rare case that 'du' produced
> no output. But they noticed it upon testing on Rawhide because after a
> normal invocation line[0] == '\0' instead of having the last line.
>
> Since the original standardization of getline/getdelim was based on the
> existing glibc functions, Eric Blake opened a bug report with the Austin
> Group to request clarification on this [3].
>
> I'm a bit torn on which behavior is better, so others thoughts would be
> appreciated.

I think we should revert this change because it arguably has a different
bug: EOF can also mean error, and we probably shouldn't write to the
buffer in that case.

(Although there are other errors that will cause buffer writes, if the
prefix of a line has been read.)

Thanks,
Florian