[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