dd: synchronize output after write errors

Problem reported by Sworddragon (Bug#51345).
* src/dd.c (cleanup): Synchronize output unless dd has been interrupted.
(synchronize_output): New function, split out from dd_copy.
Update conversions_mask so synchronization is done at most once.
(main): Do not die with the output file open, since we want to be
able to synchronize it before exiting.  Synchronize output before
exiting.

diff --git a/NEWS b/NEWS
index 15c9428bd3dcfe5e0bc017911b8de8759d331be2..757abee1543b7d2fe7a16df665ebf1ddd82a95a6 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -41,6 +41,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   padding them with zeros to 9 digits.  It uses clock_getres and
   clock_gettime to infer the clock resolution.
 
+  dd conv=fsync now synchronizes output even after a write error,
+  and similarly for dd conv=fdatasync.
+
   timeout --foreground --kill-after=... will now exit with status 137
   if the kill signal was sent, which is consistent with the behavior
   when the --foreground option is not specified.  This allows users to

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index c174065503f6e43ccc238f81a4dd18b84d3817c0..088d1764c1c7c80407638161b45830dec3b81811 100644 (file)
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -9397,14 +9397,16 @@ Continue after read errors.
 @item fdatasync
 @opindex fdatasync
 @cindex synchronized data writes, before finishing
-Synchronize output data just before finishing.  This forces a physical
-write of output data.
+Synchronize output data just before finishing,
+even if there were write errors.
+This forces a physical write of output data.
 
 @item fsync
 @opindex fsync
 @cindex synchronized data and metadata writes, before finishing
-Synchronize output data and metadata just before finishing.  This
-forces a physical write of output data and metadata.
+Synchronize output data and metadata just before finishing,
+even if there were write errors.
+This forces a physical write of output data and metadata.
 
 @end table
 

diff --git a/src/dd.c b/src/dd.c
index 957ad129ef658a02d5f014924b37a5d22e8a9e61..4ddc6db12c810b969e94b0f77cbbff31136342b1 100644 (file)
--- a/src/dd.c
+++ b/src/dd.c
@@ -939,6 +939,8 @@ iclose (int fd)
   return 0;
 }
 
+static int synchronize_output (void);
+
 static void
 cleanup (void)
 {
@@ -948,6 +950,13 @@ cleanup (void)
   alignfree (obuf);
 #endif
 
+  if (!interrupt_signal)
+    {
+      int sync_status = synchronize_output ();
+      if (sync_status)
+        exit (sync_status);
+    }
+
   if (iclose (STDIN_FILENO) != 0)
     die (EXIT_FAILURE, errno, _("closing input file %s"), quoteaf (input_file));
 
@@ -2377,17 +2386,33 @@ dd_copy (void)
       && 0 <= reported_w_bytes && reported_w_bytes < w_bytes)
     print_xfer_stats (0);
 
-  if ((conversions_mask & C_FDATASYNC) && ifdatasync (STDOUT_FILENO) != 0)
+  return exit_status;
+}
+
+/* Synchronize output according to conversions_mask.
+   Do this even if w_bytes is zero, as fsync and fdatasync
+   flush out write requests from other processes too.
+   Clear bits in conversions_mask so that synchronization is done only once.
+   Return zero if successful, an exit status otherwise.  */
+
+static int
+synchronize_output (void)
+{
+  int exit_status = 0;
+  int mask = conversions_mask;
+  conversions_mask &= ~ (C_FDATASYNC | C_FSYNC);
+
+  if ((mask & C_FDATASYNC) && ifdatasync (STDOUT_FILENO) != 0)
     {
       if (errno != ENOSYS && errno != EINVAL)
         {
           error (0, errno, _("fdatasync failed for %s"), quoteaf (output_file));
           exit_status = EXIT_FAILURE;
         }
-      conversions_mask |= C_FSYNC;
+      mask |= C_FSYNC;
     }
 
-  if ((conversions_mask & C_FSYNC) && ifsync (STDOUT_FILENO) != 0)
+  if ((mask & C_FSYNC) && ifsync (STDOUT_FILENO) != 0)
     {
       error (0, errno, _("fsync failed for %s"), quoteaf (output_file));
       return EXIT_FAILURE;
@@ -2460,6 +2485,16 @@ main (int argc, char **argv)
            | (conversions_mask & C_EXCL ? O_EXCL : 0)
            | (seek_records || (conversions_mask & C_NOTRUNC) ? 0 : O_TRUNC));
 
+      off_t size;
+      if ((INT_MULTIPLY_WRAPV (seek_records, output_blocksize, &size)
+           || INT_ADD_WRAPV (seek_bytes, size, &size))
+          && !(conversions_mask & C_NOTRUNC))
+        die (EXIT_FAILURE, 0,
+             _("offset too large: "
+               "cannot truncate to a length of seek=%"PRIdMAX""
+               " (%td-byte) blocks"),
+             seek_records, output_blocksize);
+
       /* Open the output file with *read* access only if we might
          need to read to satisfy a 'seek=' request.  If we can't read
          the file, go ahead with write-only access; it might work.  */
@@ -2472,15 +2507,6 @@ main (int argc, char **argv)
 
       if (seek_records != 0 && !(conversions_mask & C_NOTRUNC))
         {
-          off_t size;
-          if (INT_MULTIPLY_WRAPV (seek_records, output_blocksize, &size)
-              || INT_ADD_WRAPV (seek_bytes, size, &size))
-            die (EXIT_FAILURE, 0,
-                 _("offset too large: "
-                   "cannot truncate to a length of seek=%"PRIdMAX""
-                   " (%td-byte) blocks"),
-                 seek_records, output_blocksize);
-
           if (iftruncate (STDOUT_FILENO, size) != 0)
             {
               /* Complain only when ftruncate fails on a regular file, a
@@ -2491,17 +2517,21 @@ main (int argc, char **argv)
               int ftruncate_errno = errno;
               struct stat stdout_stat;
               if (ifstat (STDOUT_FILENO, &stdout_stat) != 0)
-                die (EXIT_FAILURE, errno, _("cannot fstat %s"),
-                     quoteaf (output_file));
-              if (S_ISREG (stdout_stat.st_mode)
-                  || S_ISDIR (stdout_stat.st_mode)
-                  || S_TYPEISSHM (&stdout_stat))
+                {
+                  error (0, errno, _("cannot fstat %s"),
+                         quoteaf (output_file));
+                  exit_status = EXIT_FAILURE;
+                }
+              else if (S_ISREG (stdout_stat.st_mode)
+                       || S_ISDIR (stdout_stat.st_mode)
+                       || S_TYPEISSHM (&stdout_stat))
                 {
                   intmax_t isize = size;
-                  die (EXIT_FAILURE, ftruncate_errno,
-                       _("failed to truncate to %"PRIdMAX" bytes"
-                         " in output file %s"),
-                       isize, quoteaf (output_file));
+                  error (0, ftruncate_errno,
+                         _("failed to truncate to %"PRIdMAX" bytes"
+                           " in output file %s"),
+                         isize, quoteaf (output_file));
+                  exit_status = EXIT_FAILURE;
                 }
             }
         }
@@ -2512,6 +2542,10 @@ main (int argc, char **argv)
 
   exit_status = dd_copy ();
 
+  int sync_status = synchronize_output ();
+  if (sync_status)
+    exit_status = sync_status;
+
   if (max_records == 0 && max_bytes == 0)
     {
       /* Special case to invalidate cache to end of file.  */