[PATCH 7/8] cp: fix copying a sparse file to a pipe

[Jim Meyering]

I discovered that my recent changes had broken a corner of cp.
Here's a fix and a test to exercise the bug.

>From cc0645fc643d3ea5a06f12f5fad784a2b8097888 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 31 Jan 2011 18:25:58 +0100
Subject: [PATCH 7/8] cp: fix copying a sparse file to a pipe

The recent FIEMAP-related changes made it so the unusual case of
copying a sparse file to a non-regular destination (e.g., a pipe)
would erroneously write one byte too many to that destination.
That happened because extent_copy assumed that it could use lseek
to obtain the number of bytes written to the output file descriptor.
That was valid only for regular files.
* src/copy.c (sparse_copy): Add a parameter, to be used by extent_copy,
but not by reg_copy.  Adjust callers.
(extent_copy): Maintain new local, dest_pos, using new arg, n_read.
Don't call lseek on dest_fd; use new var, dest_pos, instead.
(copy_reg): Add unused arg.
---
 src/copy.c |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index d32a11c..8ba09e0 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -139,15 +139,18 @@ utimens_symlink (char const *file, struct timespec const 
*timespec)
    BUF must have sizeof(uintptr_t)-1 bytes of additional space
    beyond BUF[BUF_SIZE-1].
    Set *LAST_WRITE_MADE_HOLE to true if the final operation on
-   DEST_FD introduced a hole.  */
+   DEST_FD introduced a hole.  Set *TOTAL_N_READ to the number of
+   bytes read.  */
 static bool
 sparse_copy (int src_fd, int dest_fd, char *buf, size_t buf_size,
              bool make_holes,
              char const *src_name, char const *dst_name,
-             uintmax_t max_n_read, bool *last_write_made_hole)
+             uintmax_t max_n_read, off_t *total_n_read,
+             bool *last_write_made_hole)
 {
   typedef uintptr_t word;
   *last_write_made_hole = false;
+  *total_n_read = 0;

   while (max_n_read)
     {
@@ -164,6 +167,7 @@ sparse_copy (int src_fd, int dest_fd, char *buf, size_t 
buf_size,
       if (n_read == 0)
         break;
       max_n_read -= n_read;
+      *total_n_read += n_read;

       if (make_holes)
         {
@@ -313,6 +317,10 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t 
buf_size,
   off_t last_ext_start = 0;
   uint64_t last_ext_len = 0;

+  /* Keep track of the output position.
+     We may need this at the end, for a final ftruncate.  */
+  off_t dest_pos = 0;
+
   extent_scan_init (src_fd, &scan);

   bool wrote_hole_at_eof = true;
@@ -378,10 +386,14 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t 
buf_size,
           last_ext_start = ext_start;
           last_ext_len = ext_len;

+          off_t n_read;
           if ( ! sparse_copy (src_fd, dest_fd, buf, buf_size,
-                              make_holes, src_name, dst_name, ext_len,
+                              make_holes, src_name, dst_name,
+                              ext_len, &n_read,
                               &wrote_hole_at_eof))
             return false;
+
+          dest_pos = ext_start + n_read;
         }

       /* Release the space allocated to scan->ext_info.  */
@@ -398,11 +410,10 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t 
buf_size,
      In addition, if the final extent was a block of zeros at EOF and we've
      just converted them to a hole in the destination, we must call ftruncate
      here in order to record the proper length in the destination.  */
-  off_t dest_len = lseek (dest_fd, 0, SEEK_CUR);
-  if ((dest_len < src_total_size || wrote_hole_at_eof)
+  if ((dest_pos < src_total_size || wrote_hole_at_eof)
       && (make_holes
           ? ftruncate (dest_fd, src_total_size)
-          : ! write_zeros (dest_fd, src_total_size - dest_len)))
+          : ! write_zeros (dest_fd, src_total_size - dest_pos)))
     {
       error (0, errno, _("failed to extend %s"), quote (dst_name));
       return false;
@@ -981,9 +992,11 @@ copy_reg (char const *src_name, char const *dst_name,
           goto close_src_and_dst_desc;
         }

+      off_t n_read;
       bool wrote_hole_at_eof;
       if ( ! sparse_copy (source_desc, dest_desc, buf, buf_size,
-                          make_holes, src_name, dst_name, UINTMAX_MAX,
+                          make_holes, src_name, dst_name,
+                          UINTMAX_MAX, &n_read,
                           &wrote_hole_at_eof)
            || (wrote_hole_at_eof &&
                ! sparse_copy_finalize (dest_desc, dst_name)))
--
1.7.3.5.44.g960a


>From 43d739e99c508a7c0fdfabc3c1aae844b029766f Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 31 Jan 2011 17:18:13 +0100
Subject: [PATCH 8/8] tests: exercise a rarely-used corner of copy.c

* tests/cp/sparse-to-pipe: New test.
* tests/Makefile.am (TESTS): Add it.
---
 tests/Makefile.am       |    1 +
 tests/cp/sparse-to-pipe |   31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 0 deletions(-)
 create mode 100755 tests/cp/sparse-to-pipe

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 40d35ac..751b327 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -343,6 +343,7 @@ TESTS =                                             \
   cp/same-file                                 \
   cp/slink-2-slink                             \
   cp/sparse                                    \
+  cp/sparse-to-pipe                            \
   cp/special-f                                 \
   cp/src-base-dot                              \
   cp/symlink-slash                             \
diff --git a/tests/cp/sparse-to-pipe b/tests/cp/sparse-to-pipe
new file mode 100755
index 0000000..4d39458
--- /dev/null
+++ b/tests/cp/sparse-to-pipe
@@ -0,0 +1,31 @@
+#!/bin/sh
+# copy a sparse file to a pipe, to exercise some seldom-used parts of copy.c
+
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+print_ver_ cp
+
+require_sparse_support_
+
+mkfifo_or_skip_ pipe
+timeout 10 cat pipe > copy &
+
+truncate -s1M sparse || framework_failure_
+cp sparse pipe || fail=1
+cmp sparse copy || fail=1
+
+Exit $fail
--
1.7.3.5.44.g960a