[PATCH v2] libio: null terminate the buffer upon initial allocation in getdelim

Eric Blake eblake@redhat.com
Thu Nov 27 22:53:45 GMT 2025


On Thu, Nov 27, 2025 at 01:13:43PM -0800, Collin Funk wrote:
> Commit 33eff78c8b28adc4963987880e10d96761f2a167 caused issues in nbdkit
> which had code similar to this to get the last line of the file:
> 
>     while (getline (&line, &len, fp) != -1)
>       ;
>     /* Process LINE.  */
> 
> After that commit, line[0] would be equal to '\0' instead of containing
> the last line of the file like before that commit.
> 
> This patch null terminates the buffer upon getdelim/getline's initial
> allocation. This is compatible with previous glibc versions, while also
> protecting the caller from reading uninitialized memory if the file is
> empty, as long as getline/getdelim does the initial allocation.

Worth calling out the link to the POSIX discussion here in the commit
message how behavior on -1 is unspecified, and therefore we are okay
doing this?  (I see you do it in the patch proper, but that's harder
to see when just perusing git log)

> ---
>  libio/iogetdelim.c   |  7 +++++--
>  libio/tst-getdelim.c | 36 +++++++++++++++++++++++++++++-------
>  manual/stdio.texi    |  7 ++++++-
>  3 files changed, 40 insertions(+), 10 deletions(-)
> 
> +  /* Test that we can read until -1 is returned and then access the last line
> +     of the file.  This behavior was broken by commit
> +     33eff78c8b28adc4963987880e10d96761f2a167 and later fixed.  */
> +  lineptr = NULL;
> +  linelen = 0;
> +  TEST_VERIFY_EXIT (fwrite ("a\nb\nc\n", 1, sizeof "a\nb\nc\n" - 1, fp)
> +                    == sizeof "a\nb\nc\n" - 1);
> +  TEST_VERIFY_EXIT (0 <= fseeko (fp, 0, SEEK_SET));
> +  char expect[] = { 'a' - 1, '\n', '\0' };
> +  for (int i = 0; i < 3; ++i)
> +    {
> +      ++expect[0];
> +      TEST_VERIFY (getdelim (&lineptr, &linelen, '\n', fp) == 2);
> +      TEST_VERIFY (linelen > 0);
> +      TEST_VERIFY (strcmp (lineptr, expect) == 0);
> +    }
> +  TEST_VERIFY (getdelim (&lineptr, &linelen, '\n', fp) == -1);
> +  TEST_VERIFY (feof (fp));
> +  TEST_VERIFY (linelen > 0);
> +  TEST_VERIFY (strcmp (lineptr, expect) == 0);

This tests a non-empty file that ends in \n.  Is it also worth testing
a non-text file that ends in a partial line?  (after all, POSIX says a
non-empty file MUST end in newline to be a text file, but that
getline/getdelim are still well-defined on non-text files).

> +++ b/manual/stdio.texi
> @@ -1279,7 +1279,12 @@ @node Line Input
>  POSIX.1-2008.
>  
>  If an error occurs or end of file is reached without any bytes read,
> -@code{getline} returns @code{-1}.
> +@code{getline} returns @code{-1}.  POSIX leaves the contents of
> +@code{*@var{lineptr}} undefined when @code{getline} returns @code{-1}.
> +If the @glibcadj{} implementation of @code{getline} allocates the
> +initial buffer it will null terminate it to prevent the caller from
> +reading uninitialized memory if no characters can be read from
> +@code{stream}.
>  @end deftypefun

The code and doc changes look reasonable to me; whether or not you
want to respin to v3 for even more unit testing is a different
question.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org



More information about the Libc-alpha mailing list