[PATCH] stdio-common: Add the fgetln function

[PATCH] stdio-common: Add the fgetln function

[Florian Weimer]

This function cannot really be implemented outside of glibc because
fclose needs to free the buffer allocated by fgetln.  Various
getline-based compatibility wrappers exist, but they are not
thread-safe because they use a global pool of buffers to avoid
an unbounded memory leak.  To prevent the use of these wrappers, it
makes sense to implement the function in glibc.

Tested on i686-linux-gnu, x86_64-linux-gnu.  Built with
build-many-glibcs.py.

---
 NEWS                                               |   3 +
 libio/bits/types/struct_FILE.h                     |   4 +-
 libio/genops.c                                     |   1 +
 libio/iofclose.c                                   |   1 +
 libio/stdio.h                                      |   9 ++
 manual/stdio.texi                                  |  32 ++++
 stdio-common/Makefile                              |   2 +
 stdio-common/Versions                              |   3 +
 stdio-common/fgetln.c                              |  82 ++++++++++
 stdio-common/tst-fgetln.c                          | 179 +++++++++++++++++++++
 sysdeps/mach/hurd/i386/libc.abilist                |   1 +
 sysdeps/unix/sysv/linux/aarch64/libc.abilist       |   1 +
 sysdeps/unix/sysv/linux/alpha/libc.abilist         |   1 +
 sysdeps/unix/sysv/linux/arc/libc.abilist           |   1 +
 sysdeps/unix/sysv/linux/arm/be/libc.abilist        |   1 +
 sysdeps/unix/sysv/linux/arm/le/libc.abilist        |   1 +
 sysdeps/unix/sysv/linux/csky/libc.abilist          |   1 +
 sysdeps/unix/sysv/linux/hppa/libc.abilist          |   1 +
 sysdeps/unix/sysv/linux/i386/libc.abilist          |   1 +
 sysdeps/unix/sysv/linux/ia64/libc.abilist          |   1 +
 sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist |   1 +
 sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist   |   1 +
 sysdeps/unix/sysv/linux/microblaze/be/libc.abilist |   1 +
 sysdeps/unix/sysv/linux/microblaze/le/libc.abilist |   1 +
 .../unix/sysv/linux/mips/mips32/fpu/libc.abilist   |   1 +
 .../unix/sysv/linux/mips/mips32/nofpu/libc.abilist |   1 +
 .../unix/sysv/linux/mips/mips64/n32/libc.abilist   |   1 +
 .../unix/sysv/linux/mips/mips64/n64/libc.abilist   |   1 +
 sysdeps/unix/sysv/linux/nios2/libc.abilist         |   1 +
 sysdeps/unix/sysv/linux/or1k/libc.abilist          |   1 +
 .../sysv/linux/powerpc/powerpc32/fpu/libc.abilist  |   1 +
 .../linux/powerpc/powerpc32/nofpu/libc.abilist     |   1 +
 .../sysv/linux/powerpc/powerpc64/be/libc.abilist   |   1 +
 .../sysv/linux/powerpc/powerpc64/le/libc.abilist   |   1 +
 sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist    |   1 +
 sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist    |   1 +
 sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist  |   1 +
 sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist  |   1 +
 sysdeps/unix/sysv/linux/sh/be/libc.abilist         |   1 +
 sysdeps/unix/sysv/linux/sh/le/libc.abilist         |   1 +
 sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist |   1 +
 sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist |   1 +
 sysdeps/unix/sysv/linux/x86_64/64/libc.abilist     |   1 +
 sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist    |   1 +
 44 files changed, 348 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index ef8ac4acd2..f3fcd4edb5 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,9 @@ Major new features:
   -z pack-relative-relocs option, which is supported for some targets
   in recent binutils versions.  Lazy binding doesn't apply to DT_RELR.
 
+* The fgetln line reading function has been added, based on the BSD
+  function of the same name.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * Support for prelink will be removed in the next release; this includes
diff --git a/libio/bits/types/struct_FILE.h b/libio/bits/types/struct_FILE.h
index 1eb429888c..732df4c73d 100644
--- a/libio/bits/types/struct_FILE.h
+++ b/libio/bits/types/struct_FILE.h
@@ -92,10 +92,10 @@ struct _IO_FILE_complete
   struct _IO_wide_data *_wide_data;
   struct _IO_FILE *_freeres_list;
   void *_freeres_buf;
-  size_t __pad5;
+  char *_fgetln_buf;
   int _mode;
   /* Make sure we don't get into trouble again.  */
-  char _unused2[15 * sizeof (int) - 4 * sizeof (void *) - sizeof (size_t)];
+  char _unused2[15 * sizeof (int) - 4 * sizeof (void *) - sizeof (char *)];
 };
 
 /* These macros are used by bits/stdio.h and internal headers.  */
diff --git a/libio/genops.c b/libio/genops.c
index 1b629eb695..f61dacaefd 100644
--- a/libio/genops.c
+++ b/libio/genops.c
@@ -585,6 +585,7 @@ _IO_no_init (FILE *fp, int flags, int orientation,
        stream.  */
     fp->_wide_data = (struct _IO_wide_data *) -1L;
   fp->_freeres_list = NULL;
+  fp->_fgetln_buf = NULL;
 }
 
 int
diff --git a/libio/iofclose.c b/libio/iofclose.c
index 4191eb8cea..5c5b3236a0 100644
--- a/libio/iofclose.c
+++ b/libio/iofclose.c
@@ -71,6 +71,7 @@ _IO_new_fclose (FILE *fp)
       if (_IO_have_backup (fp))
 	_IO_free_backup_area (fp);
     }
+  free (fp->_fgetln_buf);
   _IO_deallocate_file (fp);
   return status;
 }
diff --git a/libio/stdio.h b/libio/stdio.h
index e6425341ce..d438bd00e2 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -645,6 +645,15 @@ extern __ssize_t getdelim (char **__restrict __lineptr,
 extern __ssize_t getline (char **__restrict __lineptr,
                           size_t *__restrict __n,
                           FILE *__restrict __stream) __wur;
+#endif /* __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2) */
+
+#ifdef __USE_MISC
+/* Reads the next line from STREAM and returns a pointer to the start of
+   an array containing it.  The array is not null-terminated.  The
+   length of the array, including the line terminator if present in
+   the input, is written to *LENGTH.  Returns NULL on EOF and failure.
+   Subsequent operations on STREAM invalidate the returned pointer.  */
+extern char *fgetln (FILE *__stream, size_t *__length) __wur;
 #endif
 
 
diff --git a/manual/stdio.texi b/manual/stdio.texi
index 753c50920d..04ce4f6a0e 100644
--- a/manual/stdio.texi
+++ b/manual/stdio.texi
@@ -1365,6 +1365,38 @@ function except that it does not implicitly lock the stream.
 This function is a GNU extension.
 @end deftypefun
 
+@deftypefun {char *} fgetln (FILE *@var{stream}, size_t *@var{length})
+@standards{BSD, unistd.h}
+@safety{@mtsafe{@mtsrace{:stream}}@asunsafe{@asucorrupt{}}@acunsafe{@acucorrupt{}}}
+The @code{fgetln} function reads a line from @var{stream}, writes the
+length of the line to @code{@var{length}}, and returns a pointer to the
+start of a character array containing the line contents.  The length
+includes the line terminator if present in the stream.  (The last line
+of a file may lack a line terminator.)
+
+If the function encounters the end of stream before reading any
+characters or an error, a null pointer is returned, and zero is written
+to @code{*@var{length}}.  You can use @code{feof} and @code{ferror} to
+disambiguate the two cases.
+
+The returned pointer does not point to a string: the array is not
+null-terminated.  The pointer and the array are invalidated by a
+subsequent operation on @var{stream}.  The @code{fgetln} function does
+not perform locking.  If the caller needs locking, it has to call
+@code{flockfile} explicitly.  @xref{Streams and Threads}.
+
+The @code{fgetln} function is a BSD extension.  The comparable
+@code{getline} function is slightly less efficient on buffered streams,
+but it can be easier to use because the allocated string is
+null-terminated and not invalidated by unrelated operations on
+@var{stream}.  BSD specifies that the first @code{*@var{length}} bytes
+of the character array are writable.  In @theglibc{} implementation,
+this is not necessarily the case for some stream types, such as
+read-only memory-mapped files created by the @code{fopen} function with
+an @code{"rm"} mode argument.
+@xref{Opening Streams}.
+@end deftypefun
+
 @deftypefn {Deprecated function} {char *} gets (char *@var{s})
 @standards{ISO, stdio.h}
 @safety{@prelim{}@mtsafe{}@asunsafe{@asucorrupt{}}@acunsafe{@aculock{} @acucorrupt{}}}
diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index a1603e82fe..3577c8d8ad 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -31,6 +31,7 @@ routines := \
   ctermid \
   cuserid \
   dprintf \
+  fgetln \
   flockfile \
   fprintf \
   fscanf \
@@ -161,6 +162,7 @@ tests := \
   tst-cookie \
   tst-fdopen \
   tst-ferror \
+  tst-fgetln \
   tst-fgets \
   tst-fileno \
   tst-fmemopen \
diff --git a/stdio-common/Versions b/stdio-common/Versions
index 522f302198..c1bb41acef 100644
--- a/stdio-common/Versions
+++ b/stdio-common/Versions
@@ -63,6 +63,9 @@ libc {
   GLIBC_2.29 {
     # SHLIB_COMPAT(GLIBC_2_0, GLIBC_2_29) used in iovfscanf.c etc.
   }
+  GLIBC_2.36 {
+    fgetln;
+  }
   GLIBC_PRIVATE {
     # global variables
     _itoa_lower_digits;
diff --git a/stdio-common/fgetln.c b/stdio-common/fgetln.c
new file mode 100644
index 0000000000..70cf684f43
--- /dev/null
+++ b/stdio-common/fgetln.c
@@ -0,0 +1,82 @@
+/* Zero-copy line reading function.
+   Copyright (C) 2022 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include "libioP.h"
+#include <errno.h>
+#include <shlib-compat.h>
+#include <stddef.h>
+#include <string.h>
+
+char *
+__fgetln (FILE *fp, size_t *length)
+{
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
+  /* There is no fgetln support for old streams.  */
+  if (_IO_vtable_offset (fp) != 0)
+    {
+      *length = 0;
+      __set_errno (EOPNOTSUPP);
+      return NULL;
+    }
+#endif
+
+  /* Discard the old buffer.  This optimizes for a buffered stream,
+     with multiple lines in each buffer.  */
+  if (fp->_fgetln_buf != NULL)
+    {
+      free (fp->_fgetln_buf);
+      fp->_fgetln_buf = NULL;
+    }
+
+  /* Fast path if there is a read buffer.  */
+  if (fp->_IO_read_ptr < fp->_IO_read_end)
+    {
+      char *nl = memchr (fp->_IO_read_ptr, '\n',
+                         fp->_IO_read_end - fp->_IO_read_ptr);
+      if (nl != NULL)
+        {
+          /* The line terminator was found.  Consume the line and
+             return it.  */
+          char *result = fp->_IO_read_ptr;
+          ++nl;
+          fp->_IO_read_ptr = nl;
+          *length = nl - result;
+          return result;
+        }
+      /* Fall through to the slow path.  */
+    }
+
+  /* In the slow path, just use getdelim.  __uflow would perhaps
+     re-fill the buffer without an allocation, but it also consumes a
+     character, and that would have to be somehow merged with the rest
+     of the line (if it is not possible to unread it in the same
+     buffer).  */
+  size_t allocated = 0;
+  ssize_t ret = __getdelim (&fp->_fgetln_buf, &allocated, '\n', fp);
+  if (ret <= 0)
+    {
+      free (fp->_fgetln_buf);
+      fp->_fgetln_buf = NULL;
+      ret = 0;
+    }
+  *length = ret;
+  return fp->_fgetln_buf;
+}
+
+
+weak_alias (__fgetln, fgetln)
diff --git a/stdio-common/tst-fgetln.c b/stdio-common/tst-fgetln.c
new file mode 100644
index 0000000000..e7633a9eda
--- /dev/null
+++ b/stdio-common/tst-fgetln.c
@@ -0,0 +1,179 @@
+/* Test the fgetln function.
+   Copyright (C) 2022 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <array_length.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <stdio.h>
+#include <support/check.h>
+
+struct expected_line
+{
+  const char *contents;
+  size_t length;
+};
+
+static void
+check_lines (FILE *fp, const struct expected_line *expected)
+{
+  for (size_t i = 0; expected[i].contents != NULL; ++i)
+    {
+      size_t line_length;
+      char *line = fgetln (fp, &line_length);
+      if (line == NULL)
+        FAIL_EXIT1 ("failure to read line %zu (feof=%d ferror=%d)",
+                    i, feof (fp), ferror (fp));
+      TEST_COMPARE_BLOB (expected[i].contents, expected[i].length,
+                         line, line_length);
+    }
+
+  size_t line_length;
+  TEST_VERIFY (fgetln (fp, &line_length) == NULL);
+  TEST_VERIFY (feof (fp));
+  TEST_VERIFY (!ferror (fp));
+}
+
+static void
+check_popen (const char *command,
+             const struct expected_line *expected)
+{
+  char file_buffer[8];
+
+  for (int do_file_redirect = 0; do_file_redirect < 2; ++do_file_redirect)
+    for (int buffering = 0; buffering < 4; ++buffering)
+      {
+        FILE *fp = popen (command, "r");
+        if (fp == NULL)
+          FAIL_EXIT1 ("popen: %m");
+
+        if (do_file_redirect)
+          {
+            /* Copy the data to a temporary file, and use that stream
+               instead.  */
+            FILE *tmpfp = tmpfile ();
+            if (tmpfp == NULL)
+              FAIL_EXIT1 ("tmpfile: %m");
+            while (true)
+              {
+                int ch = fgetc (fp);
+                if (ch == EOF)
+                  {
+                    TEST_VERIFY (!ferror (fp));
+                    break;
+                  }
+                fputc (ch, tmpfp);
+                TEST_VERIFY (!ferror (fp));
+              }
+
+            TEST_COMPARE (fflush (tmpfp), 0);
+            rewind (tmpfp);
+            TEST_COMPARE (pclose (fp), 0);
+            fp = tmpfp;
+          }
+
+        switch (buffering)
+          {
+          case 0:
+            /* Use default.  */
+            break;
+          case 1:
+            /* Unbuffered.  */
+            TEST_COMPARE (setvbuf (fp, NULL, _IONBF, 0), 0);
+            break;
+
+          case 2:
+            /* Small buffer.  */
+            TEST_COMPARE (setvbuf (fp, file_buffer, _IOFBF,
+                                   sizeof (file_buffer) / 2),
+                          0);
+            break;
+          case 3:
+            /* Slightly larger buffer.  */
+            TEST_COMPARE (setvbuf (fp, file_buffer, _IOFBF,
+                                   sizeof (file_buffer)),
+                          0);
+            break;
+          }
+
+        check_lines (fp, expected);
+
+        if (do_file_redirect)
+          TEST_COMPARE (fclose (fp), 0);
+        else
+          TEST_COMPARE (pclose (fp), 0);
+      }
+}
+
+static int
+do_test (void)
+{
+  check_popen ("exit 0", (const struct expected_line[]) { { } });
+  check_popen ("echo", (const struct expected_line[])
+               {
+                 { "\n", 1 },
+                 { }
+               });
+  check_popen ("echo Abc; echo Defg", (const struct expected_line[])
+               {
+                 { "Abc\n", 4 },
+                 { "Defg\n", 5 },
+                 { }
+               });
+  check_popen ("printf 'aBc\ndEfg'", (const struct expected_line[])
+               {
+                 { "aBc\n", 4 },
+                 { "dEfg", 4 },
+                 { }
+               });
+  check_popen ("printf 'abC123\ndeFg'", (const struct expected_line[])
+               {
+                 { "abC123\n", 7 },
+                 { "deFg", 4 },
+                 { }
+               });
+  check_popen ("printf 'AbC123\nDeFg4\n56\n'", (const struct expected_line[])
+               {
+                 { "AbC123\n", 7 },
+                 { "DeFg4\n", 6 },
+                 { "56\n", 3 },
+                 { }
+               });
+  check_popen ("printf 'AbC123.\nDeFg4.\n56.\n78.\n'",
+               (const struct expected_line[])
+               {
+                 { "AbC123.\n", 8 },
+                 { "DeFg4.\n", 7 },
+                 { "56.\n", 4 },
+                 { "78.\n", 4 },
+                 { }
+               });
+   check_popen ("printf 'ABC123.\nDEF\\000g4.\n56.\n78.\n'",
+               (const struct expected_line[])
+               {
+                 { "ABC123.\n", 8 },
+                 { "DEF\000g4.\n", 8 },
+                 { "56.\n", 4 },
+                 { "78.\n", 4 },
+                 { }
+               });
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/mach/hurd/i386/libc.abilist b/sysdeps/mach/hurd/i386/libc.abilist
index 4dc87e9061..3821886da2 100644
--- a/sysdeps/mach/hurd/i386/libc.abilist
+++ b/sysdeps/mach/hurd/i386/libc.abilist
@@ -2289,6 +2289,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 close_range F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/aarch64/libc.abilist b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
index 1b63d9e447..0e09f9a07c 100644
--- a/sysdeps/unix/sysv/linux/aarch64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
@@ -2616,3 +2616,4 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
diff --git a/sysdeps/unix/sysv/linux/alpha/libc.abilist b/sysdeps/unix/sysv/linux/alpha/libc.abilist
index e7e4cf7d2a..c1611d7328 100644
--- a/sysdeps/unix/sysv/linux/alpha/libc.abilist
+++ b/sysdeps/unix/sysv/linux/alpha/libc.abilist
@@ -2713,6 +2713,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/arc/libc.abilist b/sysdeps/unix/sysv/linux/arc/libc.abilist
index bc3d228e31..23fc47a635 100644
--- a/sysdeps/unix/sysv/linux/arc/libc.abilist
+++ b/sysdeps/unix/sysv/linux/arc/libc.abilist
@@ -2377,3 +2377,4 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
diff --git a/sysdeps/unix/sysv/linux/arm/be/libc.abilist b/sysdeps/unix/sysv/linux/arm/be/libc.abilist
index db7039c4ab..dffe4a4c0c 100644
--- a/sysdeps/unix/sysv/linux/arm/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/arm/be/libc.abilist
@@ -496,6 +496,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 _Exit F
 GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
 GLIBC_2.4 _IO_2_1_stdin_ D 0xa0
diff --git a/sysdeps/unix/sysv/linux/arm/le/libc.abilist b/sysdeps/unix/sysv/linux/arm/le/libc.abilist
index d2add4fb49..2124ee46d7 100644
--- a/sysdeps/unix/sysv/linux/arm/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/arm/le/libc.abilist
@@ -493,6 +493,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 _Exit F
 GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
 GLIBC_2.4 _IO_2_1_stdin_ D 0xa0
diff --git a/sysdeps/unix/sysv/linux/csky/libc.abilist b/sysdeps/unix/sysv/linux/csky/libc.abilist
index 355d72a30c..901746d3ad 100644
--- a/sysdeps/unix/sysv/linux/csky/libc.abilist
+++ b/sysdeps/unix/sysv/linux/csky/libc.abilist
@@ -2652,3 +2652,4 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
diff --git a/sysdeps/unix/sysv/linux/hppa/libc.abilist b/sysdeps/unix/sysv/linux/hppa/libc.abilist
index 3df39bb28c..be6dda0b5c 100644
--- a/sysdeps/unix/sysv/linux/hppa/libc.abilist
+++ b/sysdeps/unix/sysv/linux/hppa/libc.abilist
@@ -2601,6 +2601,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist
index c4da358f80..890ba30894 100644
--- a/sysdeps/unix/sysv/linux/i386/libc.abilist
+++ b/sysdeps/unix/sysv/linux/i386/libc.abilist
@@ -2785,6 +2785,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/ia64/libc.abilist b/sysdeps/unix/sysv/linux/ia64/libc.abilist
index 241bac70ea..7d9b2c75c0 100644
--- a/sysdeps/unix/sysv/linux/ia64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/ia64/libc.abilist
@@ -2551,6 +2551,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
index 78bf372b72..7414ca5958 100644
--- a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
@@ -497,6 +497,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 _Exit F
 GLIBC_2.4 _IO_2_1_stderr_ D 0x98
 GLIBC_2.4 _IO_2_1_stdin_ D 0x98
diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
index 00df5c901f..e778ca0c85 100644
--- a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
@@ -2728,6 +2728,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
index e8118569c3..0dee439dcf 100644
--- a/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/microblaze/be/libc.abilist
@@ -2701,3 +2701,4 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
diff --git a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
index c0d2373e64..b29a1bf658 100644
--- a/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/microblaze/le/libc.abilist
@@ -2698,3 +2698,4 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
index 2d0fd04f54..7a8c35382f 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
@@ -2693,6 +2693,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
index e39ccfb312..57c6d61ef3 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
@@ -2691,6 +2691,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
index 1e900f86e4..aa7d911708 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
@@ -2699,6 +2699,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
index 9145ba7931..718dde5d74 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
@@ -2602,6 +2602,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/nios2/libc.abilist b/sysdeps/unix/sysv/linux/nios2/libc.abilist
index e95d60d926..e84a1dae24 100644
--- a/sysdeps/unix/sysv/linux/nios2/libc.abilist
+++ b/sysdeps/unix/sysv/linux/nios2/libc.abilist
@@ -2740,3 +2740,4 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
diff --git a/sysdeps/unix/sysv/linux/or1k/libc.abilist b/sysdeps/unix/sysv/linux/or1k/libc.abilist
index ca934e374b..c6d70f8605 100644
--- a/sysdeps/unix/sysv/linux/or1k/libc.abilist
+++ b/sysdeps/unix/sysv/linux/or1k/libc.abilist
@@ -2123,3 +2123,4 @@ GLIBC_2.35 wprintf F
 GLIBC_2.35 write F
 GLIBC_2.35 writev F
 GLIBC_2.35 wscanf F
+GLIBC_2.36 fgetln F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
index 3820b9f235..4d980a48af 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
@@ -2755,6 +2755,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
index 464dc27fcd..6e3dc8c2af 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
@@ -2788,6 +2788,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
index 2f7e58747f..8589c86e8d 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
@@ -2510,6 +2510,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
index 4f3043d913..775b77c5b0 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
@@ -2812,3 +2812,4 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
index 84b6ac815a..d7c5700468 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
@@ -2379,3 +2379,4 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
index 4d5c19c56a..c3649699e6 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
@@ -2579,3 +2579,4 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
index 7c5ee8d569..6689b28a7b 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
@@ -2753,6 +2753,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
index 50de0b46cf..95e55f8366 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
@@ -2547,6 +2547,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/sh/be/libc.abilist b/sysdeps/unix/sysv/linux/sh/be/libc.abilist
index 66fba013ca..94d7b43bce 100644
--- a/sysdeps/unix/sysv/linux/sh/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sh/be/libc.abilist
@@ -2608,6 +2608,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/sh/le/libc.abilist b/sysdeps/unix/sysv/linux/sh/le/libc.abilist
index 38703f8aa0..dd348b1bd5 100644
--- a/sysdeps/unix/sysv/linux/sh/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sh/le/libc.abilist
@@ -2605,6 +2605,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
index 6df55eb765..821ed2415b 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
@@ -2748,6 +2748,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 _IO_fprintf F
 GLIBC_2.4 _IO_printf F
 GLIBC_2.4 _IO_sprintf F
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
index b90569d881..4ee94b6df9 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
@@ -2574,6 +2574,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
index e88b0f101f..bb31ad0d09 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
@@ -2525,6 +2525,7 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F
 GLIBC_2.4 __confstr_chk F
 GLIBC_2.4 __fgets_chk F
 GLIBC_2.4 __fgets_unlocked_chk F
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
index e0755272eb..b458cae553 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
@@ -2631,3 +2631,4 @@ GLIBC_2.35 __memcmpeq F
 GLIBC_2.35 _dl_find_object F
 GLIBC_2.35 epoll_pwait2 F
 GLIBC_2.35 posix_spawn_file_actions_addtcsetpgrp_np F
+GLIBC_2.36 fgetln F

Re: [PATCH] stdio-common: Add the fgetln function

[Andreas Schwab]

On Mai 03 2022, Florian Weimer via Libc-alpha wrote:

> +/* Reads the next line from STREAM and returns a pointer to the start of
      Read                                return

> +   an array containing it.  The array is not null-terminated.  The

That makes it a rather clunky interface.  Why do we need that, given
getline et al?

> +   length of the array, including the line terminator if present in
> +   the input, is written to *LENGTH.  Returns NULL on EOF and failure.
                                         Return

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] stdio-common: Add the fgetln function

[Florian Weimer]

* Andreas Schwab:

> On Mai 03 2022, Florian Weimer via Libc-alpha wrote:
>
>> +/* Reads the next line from STREAM and returns a pointer to the start of
>       Read                                return
>
>> +   an array containing it.  The array is not null-terminated.  The
>
> That makes it a rather clunky interface.  Why do we need that, given
> getline et al?

Unfortunately, some software uses this function.  It cannot be reliably
implemented outside of libc, and there are some really problematic
portability wrappers out there.

For example, the libbsd implementation is not thread-safe, and has a
use-after-free condition if more than 32 streams are used:

| static struct filebuf fb_pool[FILEBUF_POOL_ITEMS];
| static int fb_pool_cur;
[…]
|
|	/* Try to diminish the possibility of several fgetln() calls being
|	 * used on different streams, by using a pool of buffers per file. */
|	fb = &fb_pool[fb_pool_cur];
|	if (fb->fp != stream && fb->fp != NULL) {
|		fb_pool_cur++;
|		fb_pool_cur %= FILEBUF_POOL_ITEMS;
|		fb = &fb_pool[fb_pool_cur];
|	}
|	fb->fp = stream;

<https://github.com/freedesktop/libbsd/blob/master/src/fgetln.c>

Thanks,
Florian

Re: [PATCH] stdio-common: Add the fgetln function

[Andreas Schwab]

On Mai 03 2022, Florian Weimer wrote:

> * Andreas Schwab:
>
>> On Mai 03 2022, Florian Weimer via Libc-alpha wrote:
>>
>>> +/* Reads the next line from STREAM and returns a pointer to the start of
>>       Read                                return
>>
>>> +   an array containing it.  The array is not null-terminated.  The
>>
>> That makes it a rather clunky interface.  Why do we need that, given
>> getline et al?
>
> Unfortunately, some software uses this function.

getline is standard.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] stdio-common: Add the fgetln function

[Florian Weimer]

* Andreas Schwab:

> On Mai 03 2022, Florian Weimer wrote:
>
>> * Andreas Schwab:
>>
>>> On Mai 03 2022, Florian Weimer via Libc-alpha wrote:
>>>
>>>> +/* Reads the next line from STREAM and returns a pointer to the start of
>>>       Read                                return
>>>
>>>> +   an array containing it.  The array is not null-terminated.  The
>>>
>>> That makes it a rather clunky interface.  Why do we need that, given
>>> getline et al?
>>
>> Unfortunately, some software uses this function.
>
> getline is standard.

I think we should provide this function to avoid the bugs I outlined.

Thanks,
Florian

Re: [PATCH] stdio-common: Add the fgetln function

[Andreas Schwab]

On Mai 03 2022, Florian Weimer wrote:

> I think we should provide this function to avoid the bugs I outlined.

I don't think we should encourage the use of a clunky interface if a
much superior and standard one exists.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] stdio-common: Add the fgetln function

[Cristian Rodríguez]

On Tue, May 3, 2022 at 5:01 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:

> I think we should provide this function to avoid the bugs I outlined.

For what my opinion is worth..I agree  most relevant BSD functions
that are not implemented in glibc should be..
not because they are nice, in fact some of this interfaces are
terrible and their use should not be encouraged,
however the sad reality is there is a ton of diverging, buggy "compat
wrappers" around just to deal with glibc not having this functions..
just look at openssh as a start..

Re: [PATCH] stdio-common: Add the fgetln function

[Paul Eggert]

If the stream is not already oriented, FreeBSD getln sets the stream to 
byte-orientation. Should glibc getln do the same?

On 5/3/22 00:36, Florian Weimer via Libc-alpha wrote:
> +  /* Discard the old buffer.  This optimizes for a buffered stream,
> +     with multiple lines in each buffer.  */
> +  if (fp->_fgetln_buf != NULL)
> +    {
> +      free (fp->_fgetln_buf);
> +      fp->_fgetln_buf = NULL;
> +    }

Hope you don't mind a bit of bikeshedding here....

Why free the fgetln buffer eagerly? Instead, free it only when closing. 
That would lessen pressure on the memory allocator and would save a few 
insns in fgetln's usual case.

Come to think of it, how about if we restrict fgetln to streams for 
which either (1) the user has not called setvbuf with a nonnull buffer, 
or (2) the input line fits in the user-supplied setvbuf buffer.  Then we 
wouldn't need to worry about adding a _fgetln_buf slot, as fgetln could 
always return a pointer into the already-existing stdio buffer, possibly 
by enlarging the buffer in case (1). (In case (2) fgetln could fail with 
ENOMEM if the input line is longer than the user-supplied buffer.) This 
would suffice for 99.9% of applications and would be more efficient than 
what FreeBSD does, and the whole point of fgetln is low-level efficiency 
right?

Re: [PATCH] stdio-common: Add the fgetln function

[Florian Weimer]

* Paul Eggert:

> If the stream is not already oriented, FreeBSD getln sets the stream
> to byte-orientation. Should glibc getln do the same?

Our getdelim doesn't do that explicitly.

> On 5/3/22 00:36, Florian Weimer via Libc-alpha wrote:
>> +  /* Discard the old buffer.  This optimizes for a buffered stream,
>> +     with multiple lines in each buffer.  */
>> +  if (fp->_fgetln_buf != NULL)
>> +    {
>> +      free (fp->_fgetln_buf);
>> +      fp->_fgetln_buf = NULL;
>> +    }
>
> Hope you don't mind a bit of bikeshedding here....
>
> Why free the fgetln buffer eagerly? Instead, free it only when
> closing. That would lessen pressure on the memory allocator and would
> save a few insns in fgetln's usual case.

The assumption is that very few lines cross buffer boundaries.

> Come to think of it, how about if we restrict fgetln to streams for
> which either (1) the user has not called setvbuf with a nonnull
> buffer, or (2) the input line fits in the user-supplied setvbuf
> buffer.

It would require a layering violation as far as libio is concerned: a
high-level function such as fgetln cannot reallocate the read buffer.
You mention setvbuf, but there are probably other cases (and of course
GCC 2.95 C++ classes, but we don't need to worry about compatibility
with those, I think).

> Then we wouldn't need to worry about adding a _fgetln_buf
> slot, as fgetln could always return a pointer into the
> already-existing stdio buffer, possibly by enlarging the buffer in
> case (1). (In case (2) fgetln could fail with ENOMEM if the input line
> is longer than the user-supplied buffer.) This would suffice for 99.9%
> of applications and would be more efficient than what FreeBSD does,
> and the whole point of fgetln is low-level efficiency right?

I'm not sure if it's more efficient.  The I/O block granularity would
change depending on where lines end.

Thanks,
Florian

Re: [PATCH] stdio-common: Add the fgetln function

[Paul Eggert]

On 6/9/22 00:37, Florian Weimer wrote:
> * Paul Eggert:
> 
>> If the stream is not already oriented, FreeBSD getln sets the stream
>> to byte-orientation. Should glibc getln do the same?
> 
> Our getdelim doesn't do that explicitly.

I raised the issue because one motivation for adding fgetln is to be 
compatible with FreeBSD. Although the orientation issue is secondary and 
can be detached from the main issue of adding fgetln, it might be 
helpful to address it while fgetln is being added (assuming it's added) 
rather than later.

Perhaps we'll decide that neither fgetln nor getdelim should change 
orientation, i.e., we're deliberately incompatible with FreeBSD. That's 
OK too.


>> On 5/3/22 00:36, Florian Weimer via Libc-alpha wrote:
>>> +  /* Discard the old buffer.  This optimizes for a buffered stream,
>>> +     with multiple lines in each buffer.  */
>>> +  if (fp->_fgetln_buf != NULL)
>>> +    {
>>> +      free (fp->_fgetln_buf);
>>> +      fp->_fgetln_buf = NULL;
>>> +    }
>>
>> Hope you don't mind a bit of bikeshedding here....
>>
>> Why free the fgetln buffer eagerly? Instead, free it only when
>> closing. That would lessen pressure on the memory allocator and would
>> save a few insns in fgetln's usual case.
> 
> The assumption is that very few lines cross buffer boundaries.

Yes, but the above test is run every time fgetln is called. It's faster 
to free only when closing, even if no lines cross buffer boundaries. The 
code must free when closing anyway, so this doesn't slow down closing. 
(But see below, as we may not need that buffer at all.)


>> Come to think of it, how about if we restrict fgetln to streams for
>> which either (1) the user has not called setvbuf with a nonnull
>> buffer, or (2) the input line fits in the user-supplied setvbuf
>> buffer.
> 
> It would require a layering violation as far as libio is concerned: a
> high-level function such as fgetln cannot reallocate the read buffer.
> You mention setvbuf, but there are probably other cases (and of course
> GCC 2.95 C++ classes, but we don't need to worry about compatibility
> with those, I think).

It's OK to violate layering if the performance improvement is worth it, 
which I would guess it would be for apps reading long lines.


> I'm not sure if it's more efficient.  The I/O block granularity would
> change depending on where lines end.

Can't we arrange for I/O blocking to be respected as the buffer grows? 
fgetln shouldn't need to stop reading the instant it sees a newline; it 
can read with the same blocksize it always does.

My sense is that a one-buffer solution is more efficient than two 
buffers, where data are copied from one into the other. Of course I 
haven't measured this though.

Re: [PATCH] stdio-common: Add the fgetln function

[Florian Weimer]

* Paul Eggert:

> On 6/9/22 00:37, Florian Weimer wrote:
>> * Paul Eggert:
>> 
>>> If the stream is not already oriented, FreeBSD getln sets the stream
>>> to byte-orientation. Should glibc getln do the same?
>> Our getdelim doesn't do that explicitly.
>
> I raised the issue because one motivation for adding fgetln is to be
> compatible with FreeBSD. Although the orientation issue is secondary
> and can be detached from the main issue of adding fgetln, it might be 
> helpful to address it while fgetln is being added (assuming it's
> added) rather than later.
>
> Perhaps we'll decide that neither fgetln nor getdelim should change
> orientation, i.e., we're deliberately incompatible with
> FreeBSD. That's OK too.

I will think about it.

>> I'm not sure if it's more efficient.  The I/O block granularity would
>> change depending on where lines end.
>
> Can't we arrange for I/O blocking to be respected as the buffer grows?
> fgetln shouldn't need to stop reading the instant it sees a newline;
> it can read with the same blocksize it always does.
>
> My sense is that a one-buffer solution is more efficient than two
> buffers, where data are copied from one into the other. Of course I 
> haven't measured this though.

I'm not sure if there is a good allocation scheme for this that is
obviously superior to a separate allocation.  If the end of the line
crosses the buffer boundary for the first time, moving the line to the
start of the buffer does not gives of sufficient room for a full block,
so we have to grow the buffer, by at least the number of bytes in the
line prefix read so far.  Not sure if we need some exponential resizing
policy there.  Assuming that the line is reasonably long, we will then
find a line terminator in the newly read block, and can return a pointer
to the start of the buffer from fgetln.

But we would have to teach the rest of libio to avoid the extra buffer
space at the end during future read operations.  We could avoid these
changes if we resized the buffer to twice the original buffer size.
Then we'd still maintain buffer read alignment, just with a larger
buffer.  But that runs counter to the goal of avoiding extra
allocations.

Thanks,
Florian

Re: [PATCH] stdio-common: Add the fgetln function

[Paul Eggert]

On 6/24/22 06:01, Florian Weimer wrote:
> We could avoid these
> changes if we resized the buffer to twice the original buffer size.
> Then we'd still maintain buffer read alignment, just with a larger
> buffer.  But that runs counter to the goal of avoiding extra
> allocations.

I'm not quite following. Since there would be just one buffer, that's 
one less memory allocation than the two buffers in the proposal. Yes, 
this single buffer might waste more total space due to the memory 
allocated between the buffer start and the line start. But I doubt 
whether it's worth the CPU time and complexity and data movement merely 
to avoid that space wastage.

If the worry is that a 2x growth factor is overkill, we use a 1.5x 
growth factor (as in Gnulib's xpalloc function). It should be easy to 
maintain buffer read alignment regardless of growth factor.