Home Original page

[PING][PATCH] binutils: Avoid renaming over existing files

Alan Modra amodra@gmail.com
Tue Feb 23 00:00:36 GMT 2021 
On Mon, Feb 22, 2021 at 09:00:42PM +0530, Siddhesh Poyarekar wrote:
> On 2/19/21 10:15 PM, Nick Clifton wrote:
> > Hi Siddhesh,
> > 
> > > I don't know.  Nick, Alan, should this be backported to 2.36?
> > 
> > Yes please.
> 
> Thanks, backported and pushed to binutils-2_36-branch.

See pr27456.  It seems to me that smart_rename now will always be
copying the temp file contents to the output file in the non-windows
case, and that we can make it do the same for windows too.  I'm
testing the following.

	PR 27456
	* rename.c: Tidy throughout.
	(smart_rename): Always copy.  Remove windows specific code.

diff --git a/binutils/rename.c b/binutils/rename.c
index 2ff092ee22..72a9323d72 100644
--- a/binutils/rename.c
+++ b/binutils/rename.c
@@ -24,14 +24,9 @@
 
 #ifdef HAVE_GOOD_UTIME_H
 #include <utime.h>
-#else /* ! HAVE_GOOD_UTIME_H */
-#ifdef HAVE_UTIMES
+#elif defined HAVE_UTIMES
 #include <sys/time.h>
-#endif /* HAVE_UTIMES */
-#endif /* ! HAVE_GOOD_UTIME_H */
-
-#if ! defined (_WIN32) || defined (__CYGWIN32__)
-static int simple_copy (const char *, const char *);
+#endif
 
 /* The number of bytes to copy at once.  */
 #define COPY_BUF 8192
@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
     }
   return 0;
 }
-#endif /* __CYGWIN32__ or not _WIN32 */
 
 /* Set the times of the file DESTINATION to be the same as those in
    STATBUF.  */
@@ -91,87 +85,52 @@ void
 set_times (const char *destination, const struct stat *statbuf)
 {
   int result;
-
-  {
 #ifdef HAVE_GOOD_UTIME_H
-    struct utimbuf tb;
-
-    tb.actime = statbuf->st_atime;
-    tb.modtime = statbuf->st_mtime;
-    result = utime (destination, &tb);
-#else /* ! HAVE_GOOD_UTIME_H */
-#ifndef HAVE_UTIMES
-    long tb[2];
-
-    tb[0] = statbuf->st_atime;
-    tb[1] = statbuf->st_mtime;
-    result = utime (destination, tb);
-#else /* HAVE_UTIMES */
-    struct timeval tv[2];
-
-    tv[0].tv_sec = statbuf->st_atime;
-    tv[0].tv_usec = 0;
-    tv[1].tv_sec = statbuf->st_mtime;
-    tv[1].tv_usec = 0;
-    result = utimes (destination, tv);
-#endif /* HAVE_UTIMES */
-#endif /* ! HAVE_GOOD_UTIME_H */
-  }
+  struct utimbuf tb;
+
+  tb.actime = statbuf->st_atime;
+  tb.modtime = statbuf->st_mtime;
+  result = utime (destination, &tb);
+#elif defined HAVE_UTIMES
+  struct timeval tv[2];
+
+  tv[0].tv_sec = statbuf->st_atime;
+  tv[0].tv_usec = 0;
+  tv[1].tv_sec = statbuf->st_mtime;
+  tv[1].tv_usec = 0;
+  result = utimes (destination, tv);
+#else
+  long tb[2];
+
+  tb[0] = statbuf->st_atime;
+  tb[1] = statbuf->st_mtime;
+  result = utime (destination, tb);
+#endif
 
   if (result != 0)
     non_fatal (_("%s: cannot set time: %s"), destination, strerror (errno));
 }
 
-/* Rename FROM to TO, copying if TO exists.  TARGET_STAT has the file status
-   that, if non-NULL, is used to fix up timestamps after rename.  Return 0 if
-   ok, -1 if error.  */
+/* Copy FROM to TO.  TARGET_STAT has the file status that, if non-NULL,
+   is used to fix up timestamps.  Return 0 if ok, -1 if error.
+   At one time this function renamed files, but file permissions are
+   tricky to update given the number of different schemes used by
+   various systems.  So now we just copy.  */
 
 int
 smart_rename (const char *from, const char *to,
-	      struct stat *target_stat ATTRIBUTE_UNUSED)
+	      struct stat *target_stat)
 {
-  int ret = 0;
-  struct stat to_stat;
-  bfd_boolean exists;
-
-  exists = lstat (to, &to_stat) == 0;
-
-#if defined (_WIN32) && !defined (__CYGWIN32__)
-  /* Win32, unlike unix, will not erase `to' in `rename(from, to)' but
-     fail instead.  Also, chown is not present.  */
-
-  if (exists)
-    remove (to);
+  int ret;
 
-  ret = rename (from, to);
+  ret = simple_copy (from, to);
   if (ret != 0)
-    {
-      /* We have to clean up here.  */
-      non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
-      unlink (from);
-    }
-#else
-  /* Avoid a full copy and use rename if TO does not exist.  */
-  if (!exists)
-    {
-      if ((ret = rename (from, to)) != 0)
-	{
-	  /* We have to clean up here.  */
-	  non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
-	  unlink (from);
-	}
-    }
-  else
-    {
-      ret = simple_copy (from, to);
-      if (ret != 0)
-	non_fatal (_("unable to copy file '%s'; reason: %s"), to, strerror (errno));
+    non_fatal (_("unable to copy file '%s'; reason: %s"),
+	       to, strerror (errno));
 
-      if (target_stat != NULL)
-	set_times (to, target_stat);
-      unlink (from);
-    }
-#endif /* _WIN32 && !__CYGWIN32__ */
+  if (target_stat != NULL)
+    set_times (to, target_stat);
+  unlink (from);
 
   return ret;
 }


-- 
Alan Modra
Australia Development Lab, IBM
More information about the Binutils mailing list