Bug 207092

Description Maxim Sobolev 2016-02-10 21:25:49 UTC

The dd(1) sparse conversion is seriously broken after this change. The last block is never written at least for the case of output being regular file:

$ dd if=/dev/zero of=/tmp/foo.bar bs=1m count=10
10+0 records in
10+0 records out
10485760 bytes transferred in 0.003244 secs (3232431656 bytes/sec)
$ ktrace dd if=/tmp/foo.bar of=/tmp/foo.bar1 bs=1m conv=sparse
10+0 records in
10+0 records out
$ ls -l /tmp/foo.bar /tmp/foo.bar1
-rw-r--r--  1 sobomax  wheel  10485760 Feb  9 23:59 /tmp/foo.bar
-rw-r--r--  1 sobomax  wheel         0 Feb  9 23:59 /tmp/foo.bar1
$ uname -a
FreeBSD abc.sippysoft.com 10.3-PRERELEASE FreeBSD 10.3-PRERELEASE #1 80de3e2(master)-dirty: Tue Feb  2 12:19:57 PST 2016     sobomax@abc.sippysoft.com:/usr/obj/usr/home/sobomax/projects/freebsd103/sys/VAN01  amd64

ktrace ends with:

  3150 dd       RET   read 1048576/0x100000
  3150 dd       CALL  read(0x3,0x801009000,0x100000)
  3150 dd       GIO   fd 3 read 0 bytes
       ""
  3150 dd       RET   read 0
  3150 dd       CALL  lseek(0x4,0x900000,SEEK_CUR)
  3150 dd       RET   lseek 9437184/0x900000
  3150 dd       CALL  close(0x4)
  3150 dd       RET   close 0
  3150 dd       CALL  write(0x2,0x7fffffffe2c0,0x21)
  3150 dd       GIO   fd 2 wrote 33 bytes
       "10+0 records in
        10+0 records out
       "
  3150 dd       RET   write 33/0x21
  3150 dd       CALL  write(0x2,0x7fffffffe2c0,0x43)
  3150 dd       GIO   fd 2 wrote 67 bytes
       "10485760 bytes transferred in 0.008217 secs (1276090675 bytes/sec)
       "
  3150 dd       RET   write 67/0x43
  3150 dd       CALL  sigprocmask(SIG_BLOCK,0x800822a38,0x7fffffffe780)
  3150 dd       RET   sigprocmask 0
  3150 dd       CALL  sigprocmask(SIG_SETMASK,0x800822a4c,0)
  3150 dd       RET   sigprocmask 0
  3150 dd       CALL  sigprocmask(SIG_BLOCK,0x800822a38,0x7fffffffe310)
  3150 dd       RET   sigprocmask 0
  3150 dd       CALL  sigprocmask(SIG_SETMASK,0x800822a4c,0)
  3150 dd       RET   sigprocmask 0
  3150 dd       CALL  sigprocmask(SIG_BLOCK,0x800822a38,0x7fffffffe310)
  3150 dd       RET   sigprocmask 0
  3150 dd       CALL  sigprocmask(SIG_SETMASK,0x800822a4c,0)
  3150 dd       RET   sigprocmask 0
  3150 dd       CALL  exit(0)


Looking at the code in question I don't see how could it have worked. Look at the following piece of code in your diff for example:

+                                       if (force && cnt == 0) {
+                                               pending -= last_sp;
+                                               assert(outp == out.db);
+                                               memset(outp, 0, cnt);
+                                       }

When the branch is taken, cnt is 0, so at the very least memset(x, y, 0) is NOP.  Later on, write(2) is conditional on cnt != 0, so that it's never taken. As a result, lseek is the last operation the file sees.

Also, for what it's worth, you can use ftruncate(2) instead of write() for regular sparse files to ensure correct size. That would write just as much data as needed to the end. I've made a quick and dirty patch, that seems to be working better than current code at least:

http://sobomax.sippysoft.com/dd.diff