Re: [PATCH] copy-file-range: work around Linux kernel bug

2022-01-16 Thread Pádraig Brady

On 16/01/2022 18:06, Paul Eggert wrote:

On 1/16/22 05:37, Pádraig Brady wrote:

This looks like the replacement will only be used when the build system
uses an older kernel?
If so this seems brittle. Consider the case where el7 rpms are being
built on
central build systems with newer kernels.


It doesn't run any code on the newer kernels, right? It merely compiles.
If the compilation environment says "this is being compiled for kernel
5.3", surely the generated code can assume 5.3 or later.


Oh right. I guess the build farm would use the oldest kernel headers for this 
reason.
I.e. the replacement code would be built as long as appropriate kernel headers
were installed.  This may be a gotcha for some build / deployment setups,
but that would be unusual, so this is the right default.
I'm just extra paranoid about ensuring copy routines are robust.

Sorry for the noise.

thanks,
Pádraig



Re: [PATCH] copy-file-range: work around Linux kernel bug

2022-01-16 Thread Paul Eggert

On 1/16/22 05:37, Pádraig Brady wrote:
This looks like the replacement will only be used when the build system 
uses an older kernel?
If so this seems brittle. Consider the case where el7 rpms are being 
built on

central build systems with newer kernels.


It doesn't run any code on the newer kernels, right? It merely compiles. 
If the compilation environment says "this is being compiled for kernel 
5.3", surely the generated code can assume 5.3 or later.




Re: [PATCH] copy-file-range: work around Linux kernel bug

2022-01-16 Thread Pádraig Brady

On 15/01/2022 01:33, Paul Eggert wrote:

--- a/m4/copy-file-range.m4
+++ b/m4/copy-file-range.m4
@@ -7,6 +7,7 @@ dnl with or without modifications, as long as this notice is 
preserved.
  AC_DEFUN([gl_FUNC_COPY_FILE_RANGE],
  [
AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
+  AC_REQUIRE([AC_CANONICAL_HOST])
  
dnl Persuade glibc  to declare copy_file_range.

AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])
@@ -21,7 +22,7 @@ AC_DEFUN([gl_FUNC_COPY_FILE_RANGE],
 [AC_LANG_PROGRAM(
[[#include 
]],
-  [[ssize_t (*func) (int, off_t *, int, off_t, size_t, unsigned)
+  [[ssize_t (*func) (int, off_t *, int, off_t *, size_t, unsigned)
= copy_file_range;
  return func (0, 0, 0, 0, 0, 0) & 127;
]])
@@ -32,5 +33,27 @@ AC_DEFUN([gl_FUNC_COPY_FILE_RANGE],
  
if test "$gl_cv_func_copy_file_range" != yes; then

  HAVE_COPY_FILE_RANGE=0
+  else
+AC_DEFINE([HAVE_COPY_FILE_RANGE], 1,
+  [Define to 1 if the function copy_file_range exists.])
+
+case $host_os in
+  linux*)
+AC_CACHE_CHECK([whether copy_file_range is known to work],
+  [gl_cv_copy_file_range_known_to_work],
+  [AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM(
+[[#include 
+]],
+[[#if LINUX_VERSION_CODE < KERNEL_VERSION (5, 3, 0)
+   #error "copy_file_range is buggy"
+  #endif
+]])],
+ [gl_cv_copy_file_range_known_to_work=yes],
+ [gl_cv_copy_file_range_known_to_work=no])])
+if test "$gl_cv_copy_file_range_known_to_work" = no; then
+  REPLACE_COPY_FILE_RANGE=1
+fi;;
+esac
fi
  ])


This looks like the replacement will only be used when the build system uses an 
older kernel?
If so this seems brittle. Consider the case where el7 rpms are being built on
central build systems with newer kernels.

cheers,
Pádraig



[PATCH] copy-file-range: work around Linux kernel bug

2022-01-14 Thread Paul Eggert
This workaround is adapted from Coreutils.
* lib/copy-file-range.c [__linux__ && HAVE_COPY_FILE_RANGE]:
Include .
(copy_file_range): Use a stub to replace the copy_file_range of
Linux kernel versions 4.5 through 5.2.
* lib/unistd.in.h (copy_file_range):
* m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS):
* modules/copy-file-range (configure.ac):
* modules/unistd (unistd.h):
Support replacement of copy_file_range.
* m4/copy-file-range.m4 (gl_FUNC_COPY_FILE_RANGE):
Define HAVE_COPY_FILE_RANGE if the system has copy_file_range,
and on Linux check whether the system’s is known to work.
---
 ChangeLog| 17 
 doc/glibc-functions/copy_file_range.texi |  5 
 lib/copy-file-range.c| 34 
 lib/unistd.in.h  | 16 ++-
 m4/copy-file-range.m4| 25 -
 m4/unistd_h.m4   |  1 +
 modules/copy-file-range  |  4 ++-
 modules/unistd   |  1 +
 8 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 93fe05770b..a900fec78d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2022-01-14  Paul Eggert  
+
+   copy-file-range: work around Linux kernel bug
+   This workaround is adapted from Coreutils.
+   * lib/copy-file-range.c [__linux__ && HAVE_COPY_FILE_RANGE]:
+   Include .
+   (copy_file_range): Use a stub to replace the copy_file_range of
+   Linux kernel versions 4.5 through 5.2.
+   * lib/unistd.in.h (copy_file_range):
+   * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS):
+   * modules/copy-file-range (configure.ac):
+   * modules/unistd (unistd.h):
+   Support replacement of copy_file_range.
+   * m4/copy-file-range.m4 (gl_FUNC_COPY_FILE_RANGE):
+   Define HAVE_COPY_FILE_RANGE if the system has copy_file_range,
+   and on Linux check whether the system’s is known to work.
+
 2022-01-14  Bruno Haible  
 
Avoid error "conditional LIBUNISTRING_COMPILE_... was never defined"
diff --git a/doc/glibc-functions/copy_file_range.texi 
b/doc/glibc-functions/copy_file_range.texi
index baac6e1302..f10271c157 100644
--- a/doc/glibc-functions/copy_file_range.texi
+++ b/doc/glibc-functions/copy_file_range.texi
@@ -24,6 +24,11 @@ This function exists only on Linux and FreeBSD and is 
therefore
 missing on many non-glibc platforms:
 glibc 2.26, macOS 11.1, FreeBSD 12.0, NetBSD 9.0, OpenBSD 6.7, Minix 3.1.8, 
AIX 7.1, HP-UX 11.31, IRIX 6.5, Solaris 11.4, Cygwin 2.9, mingw, MSVC 14, 
Android 9.0.
 But the replacement function is only a stub: It always fails with error ENOSYS.
+
+@item
+This function has many problems on Linux kernel versions before 5.3.
+On these kernel versions, the replacement function always fails with
+error ENOSYS.
 @end itemize
 
 Portability problems not fixed by Gnulib:
diff --git a/lib/copy-file-range.c b/lib/copy-file-range.c
index 96f1ec7c5e..1ec7f4de67 100644
--- a/lib/copy-file-range.c
+++ b/lib/copy-file-range.c
@@ -20,11 +20,45 @@
 
 #include 
 
+#if defined __linux__ && HAVE_COPY_FILE_RANGE
+# include 
+#endif
+
 ssize_t
 copy_file_range (int infd, off_t *pinoff,
  int outfd, off_t *poutoff,
  size_t length, unsigned int flags)
 {
+#undef copy_file_range
+
+#if defined __linux__ && HAVE_COPY_FILE_RANGE
+  /* The implementation of copy_file_range (which first appeared in
+ Linux kernel release 4.5) had many issues before release 5.3
+ , so fail with ENOSYS for Linux
+ kernels 5.2 and earlier.
+
+ This workaround, and the configure-time check for Linux, can be
+ removed when such kernels (released March 2016 through September
+ 2019) are no longer a consideration.  As of January 2021, the
+ furthest-future planned kernel EOL is December 2024 for kernel
+ release 4.19.  */
+
+static signed char ok;
+
+if (! ok)
+  {
+struct utsname name;
+uname (&name);
+char *p = name.release;
+ok = ((p[1] != '.' || '5' < p[0]
+   || (p[0] == '5' && (p[3] != '.' || '2' < p[2])))
+  ? 1 : -1);
+  }
+
+if (0 < ok)
+  return copy_file_range (infd, pinoff, outfd, poutoff, length, flags);
+#endif
+
   /* There is little need to emulate copy_file_range with read+write,
  since programs that use copy_file_range must fall back on
  read+write anyway.  */
diff --git a/lib/unistd.in.h b/lib/unistd.in.h
index 3386f0b0f7..57df09ecdf 100644
--- a/lib/unistd.in.h
+++ b/lib/unistd.in.h
@@ -415,16 +415,30 @@ _GL_CXXALIASWARN (close);
 
 
 #if @GNULIB_COPY_FILE_RANGE@
-# if !@HAVE_COPY_FILE_RANGE@
+# if @REPLACE_COPY_FILE_RANGE@
+#  if !(defined __cplusplus && defined GNULIB_NAMESPACE)
+#   undef copy_file_range
+#   define copy_file_range rpl_copy_file_range
+#  endif
+_GL_FUNCDECL_RPL (copy_file_range, ssize_t, (int ifd, off_t *ipos,
+