Hello again,

Quoting Johannes Schauer (2018-12-31 10:58:18)
> I'm sorry, but my attached patch was apparently not properly tested by me. :(
> 
> More work is needed to backport the patch from upstream to version 8.30 
> because
> just calling renameat2 as proposed in the patch I attached in my first message
> will just result in an infinite recursion loop. I guess I only didn't notice
> because the initial patch never worked because HAVE_RENAMEAT2 never got 
> defined
> as the following hunk was missing from it:
> 
> --- coreutils-8.30.orig/lib/config.hin
> +++ coreutils-8.30/lib/config.hin
> @@ -2069,6 +2069,9 @@
>  /* Define to 1 if you have the `renameat' function. */
>  #undef HAVE_RENAMEAT
>  
> +/* Define to 1 if you have the `renameat2' function. */
> +#undef HAVE_RENAMEAT2
> +
>  /* Define to 1 if you have the `rewinddir' function. */
>  #undef HAVE_REWINDDIR
> 
> 
> I only now started to understand how the Debian package includes the gnulib
> submodule and ends up shipping a number of autogenerated files as well.
> 
> Upstream doesn't have the problem of infinite recursion because they renamed
> renameat2 to renameatu. So one way of solving this would be to also backport
> that change.
> 
> Would you be okay with that or would you like to see another solution?
> 
> How much time do I have to present a working patch?

I think I've got it this time. Please find attached two patches. One renames
the renameat2 function to renameatu (as it was done upstream) and the other
adds support for using glibc renameat2 instead of the systemcall (and also
includes the missing hunk from lib/config.hin).

The renameatu patch is very small because the Debian package does not seem to
run the testsuite? At least it compiles fine and seems to work fine together
with my patch to fakechroot in #909612.

Sorry to not have gotten it right when I reported the bug.

Is there anything else you'd like me to do for this bug?

Thanks!

cheers, josch
From: Johannes 'josch' Schauer <jo...@debian.org>
Date: Tue, 4 Dec 2018 20:57:48 +0100
X-Dgit-Generated: 8.30-1.1 2474a66055eceaf668b315d83ae7b0ae7bf9a4d5
Subject: Prefer renameat2 from glibc over syscall if available

This is necessary for fakechroot to be able to overwrite renameat2 which
is used by mv(1) from coreutils. See #909612

This patch is based on a patch by Andreas Henriksson <andr...@fatal.se>
which was accepted in gnulib git:

https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=c50cf67bd7ff70525f3cb4074f0d9cc1f5c6cf9c

---

--- coreutils-8.30.orig/lib/config.hin
+++ coreutils-8.30/lib/config.hin
@@ -2069,6 +2069,9 @@
 /* Define to 1 if you have the `renameat' function. */
 #undef HAVE_RENAMEAT
 
+/* Define to 1 if you have the `renameat2' function. */
+#undef HAVE_RENAMEAT2
+
 /* Define to 1 if you have the `rewinddir' function. */
 #undef HAVE_REWINDDIR
 
--- coreutils-8.30.orig/lib/renameat2.c
+++ coreutils-8.30/lib/renameat2.c
@@ -77,7 +77,10 @@ renameat2 (int fd1, char const *src, int
   int ret_val = -1;
   int err = EINVAL;
 
-#ifdef SYS_renameat2
+#if HAVE_RENAMEAT2
+  ret_val = renameat2 (fd1, src, fd2, dst, flags);
+  err = errno;
+#elif defined SYS_renameat2
   ret_val = syscall (SYS_renameat2, fd1, src, fd2, dst, flags);
   err = errno;
 #elif defined RENAME_EXCL
--- coreutils-8.30.orig/m4/renameat.m4
+++ coreutils-8.30/m4/renameat.m4
@@ -15,7 +15,7 @@ AC_DEFUN([gl_FUNC_RENAMEAT],
   AC_REQUIRE([gl_STDIO_H_DEFAULTS])
   AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS])
   AC_CHECK_HEADERS([linux/fs.h])
-  AC_CHECK_FUNCS_ONCE([renameat])
+  AC_CHECK_FUNCS_ONCE([renameat renameat2])
   if test $ac_cv_func_renameat = no; then
     HAVE_RENAMEAT=0
   elif test $REPLACE_RENAME = 1; then
From: Johannes 'josch' Schauer <jo...@debian.org>
Date: Mon, 31 Dec 2018 11:03:58 +0100
X-Dgit-Generated: 8.30-1.1 8209088c5a946519942aba2b13200c8d09b14c91
Subject: renameatu


---

--- coreutils-8.30.orig/lib/backupfile.c
+++ coreutils-8.30/lib/backupfile.c
@@ -353,7 +353,7 @@ backupfile_internal (char const *file, e
           base_offset = 0;
         }
       unsigned flags = backup_type == simple_backups ? 0 : RENAME_NOREPLACE;
-      if (renameat2 (AT_FDCWD, file, sdir, s + base_offset, flags) == 0)
+      if (renameatu (AT_FDCWD, file, sdir, s + base_offset, flags) == 0)
         break;
       int e = errno;
       if (e != EEXIST)
--- coreutils-8.30.orig/lib/renameat.c
+++ coreutils-8.30/lib/renameat.c
@@ -21,5 +21,5 @@
 int
 renameat (int fd1, char const *src, int fd2, char const *dst)
 {
-  return renameat2 (fd1, src, fd2, dst, 0);
+  return renameatu (fd1, src, fd2, dst, 0);
 }
--- coreutils-8.30.orig/lib/renameat2.c
+++ coreutils-8.30/lib/renameat2.c
@@ -71,7 +71,7 @@ rename_noreplace (char const *src, char
    function is equivalent to renameat (FD1, SRC, FD2, DST).  */
 
 int
-renameat2 (int fd1, char const *src, int fd2, char const *dst,
+renameatu (int fd1, char const *src, int fd2, char const *dst,
            unsigned int flags)
 {
   int ret_val = -1;
--- coreutils-8.30.orig/lib/renameat2.h
+++ coreutils-8.30/lib/renameat2.h
@@ -27,4 +27,4 @@
 # define RENAME_WHITEOUT   (1 << 2)
 #endif
 
-extern int renameat2 (int, char const *, int, char const *, unsigned int);
+extern int renameatu (int, char const *, int, char const *, unsigned int);

Attachment: signature.asc
Description: signature

Reply via email to