install: cleanup properly if the strip program failed for any reason

* src/install.c (strip): Indicate failure with a return code instead
of terminating the program.
(install_file_in_file): Handle strip's return code and unlink the
created file if necessary.
* tests/install/strip-program.sh: Add a test to cover the changes.
* NEWS (Bug fixes): Mention the fix.
Reported by John Reiser in http://bugzilla.redhat.com/632444.

diff --git a/NEWS b/NEWS
index 37bcdf70f818abda72bd3820768f3acc472d3760..5a253771a301fcf5247d3b25140e14c8b2305e74 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,13 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** Bug fixes
+
+  install now removes the target file if the strip program failed for any
+  reason.  Before, that file was left behind, sometimes even with wrong
+  permissions.
+  [This bug was present in "the beginning".]
+
 
 * Noteworthy changes in release 8.21 (2013-02-14) [stable]
 

diff --git a/src/install.c b/src/install.c
index 94374df3e5bb97148a82f47ea3c5f209e0f8d5c2..a5ed7a821406ecdc7477cf1fa123b37910ec2077 100644 (file)
--- a/src/install.c
+++ b/src/install.c
@@ -515,16 +515,17 @@ change_timestamps (struct stat const *src_sb, char const *dest)
    magic numbers vary so much from system to system that making
    it portable would be very difficult.  Not worth the effort. */
 
-static void
+static bool
 strip (char const *name)
 {
   int status;
+  bool ok = false;
   pid_t pid = fork ();
 
   switch (pid)
     {
     case -1:
-      error (EXIT_FAILURE, errno, _("fork system call failed"));
+      error (0, errno, _("fork system call failed"));
       break;
     case 0:                    /* Child. */
       execlp (strip_program, strip_program, name, NULL);
@@ -532,11 +533,14 @@ strip (char const *name)
       break;
     default:                   /* Parent. */
       if (waitpid (pid, &status, 0) < 0)
-        error (EXIT_FAILURE, errno, _("waiting for strip"));
+        error (0, errno, _("waiting for strip"));
       else if (! WIFEXITED (status) || WEXITSTATUS (status))
-        error (EXIT_FAILURE, 0, _("strip process terminated abnormally"));
+        error (0, 0, _("strip process terminated abnormally"));
+      else
+        ok = true;      /* strip succeeded */
       break;
     }
+  return ok;
 }
 
 /* Initialize the user and group ownership of the files to install. */
@@ -681,7 +685,12 @@ install_file_in_file (const char *from, const char *to,
   if (! copy_file (from, to, x))
     return false;
   if (strip_files)
-    strip (to);
+    if (! strip (to))
+      {
+        if (unlink (to) != 0)  /* Cleanup.  */
+          error (EXIT_FAILURE, errno, _("cannot unlink %s"), to);
+        return false;
+      }
   if (x->preserve_timestamps && (strip_files || ! S_ISREG (from_sb.st_mode))
       && ! change_timestamps (&from_sb, to))
     return false;

diff --git a/tests/install/strip-program.sh b/tests/install/strip-program.sh
index 8950d50cb8ba8c5abd03ca5d13950468cb944180..5d65373032ce4e57d9f9861decb7239690ec606d 100755 (executable)
--- a/tests/install/strip-program.sh
+++ b/tests/install/strip-program.sh
@@ -33,4 +33,8 @@ echo aBc > exp || fail=1
 ginstall src dest -s --strip-program=./b || fail=1
 compare exp dest || fail=1
 
+# Check that install cleans up properly if strip fails.
+ginstall src dest2 -s --strip-program=./FOO && fail=1
+test -e dest2 && fail=1
+
 Exit $fail