svn: head/bin/dd

Maxim Sobolev sobomax at FreeBSD.org
Fri Feb 19 06:12:57 UTC 2016


Thanks, Thomas. IMHO this is quite serious bug in one of the core
utilities, so I suggest it gets faster MFC before 10.3 goes into beta stage
on Feb 26. The old code in RELENG_10 is just plain broken anyway now, so it
could not make it any worse. Thanks!

On Thu, Feb 18, 2016 at 12:44 AM, Thomas Quinot <thomas at freebsd.org> wrote:

> Author: thomas
> Date: Thu Feb 18 08:44:16 2016
> New Revision: 295749
> URL: https://svnweb.freebsd.org/changeset/base/295749
>
> Log:
>   Reorganize the handling all-zeroes terminal block in sparse mode
>
>   The intent of the previous code in that case was to force
>   an explicit write, but the implementation was incorrect, and
>   as a result the write was never performed. This new implementation
>   instead uses ftruncate(2) to extend the file with a trailing hole.
>
>   Also introduce regression tests for these cases.
>
>   PR: 189284
>   (original PR whose fix introduced this bug)
>
>   PR: 207092
>
>   Differential Revision:        D5248
>   Reviewed by:  sobomax,kib
>   MFC after:    2 weeks
>
> Added:
>   head/bin/dd/ref.obs_zeroes   (contents, props changed)
> Modified:
>   head/bin/dd/Makefile
>   head/bin/dd/dd.c
>   head/bin/dd/dd.h
>   head/bin/dd/gen.c
>
> Modified: head/bin/dd/Makefile
>
> ==============================================================================
> --- head/bin/dd/Makefile        Thu Feb 18 07:44:14 2016        (r295748)
> +++ head/bin/dd/Makefile        Thu Feb 18 08:44:16 2016        (r295749)
> @@ -24,7 +24,18 @@ test: ${PROG} gen
>             LC_ALL=en_US.US-ASCII hexdump -C | \
>             diff -I FreeBSD - ${.CURDIR}/ref.${conv}
>  .endfor
> -       @rm -f gen
> +       @${ECHO} "testing sparse file (obs zeroes)"
> +       @./gen 189284 | ./dd ibs=16 obs=8 conv=sparse of=obs_zeroes 2>
> /dev/null
> +       @hexdump -C obs_zeroes | diff -I FreeBSD -
> ${.CURDIR}/ref.obs_zeroes
> +
> +       @${ECHO} "testing spase file (all zeroes)"
> +       @./dd if=/dev/zero of=1M_zeroes bs=1048576 count=1 2> /dev/null
> +       @./dd if=1M_zeroes of=1M_zeroes.1 bs=1048576 conv=sparse 2>
> /dev/null
> +       @./dd if=1M_zeroes of=1M_zeroes.2 bs=1048576 2> /dev/null
> +       @diff 1M_zeroes 1M_zeroes.1
> +       @diff 1M_zeroes 1M_zeroes.2
> +
> +       @rm -f gen 1M_zeroes* obs_zeroes
>
>  .if ${MK_TESTS} != "no"
>  SUBDIR+=       tests
>
> Modified: head/bin/dd/dd.c
>
> ==============================================================================
> --- head/bin/dd/dd.c    Thu Feb 18 07:44:14 2016        (r295748)
> +++ head/bin/dd/dd.c    Thu Feb 18 08:44:16 2016        (r295749)
> @@ -77,7 +77,6 @@ STAT  st;                     /* statistics */
>  void   (*cfunc)(void);         /* conversion function */
>  uintmax_t cpy_cnt;             /* # of blocks to copy */
>  static off_t   pending = 0;    /* pending seek if sparse */
> -static off_t   last_sp = 0;    /* size of last added sparse block */
>  u_int  ddflags = 0;            /* conversion options */
>  size_t cbsz;                   /* conversion block size */
>  uintmax_t files_cnt = 1;       /* # of files to copy */
> @@ -409,6 +408,15 @@ dd_close(void)
>         }
>         if (out.dbcnt || pending)
>                 dd_out(1);
> +
> +       /*
> +        * If the file ends with a hole, ftruncate it to extend its size
> +        * up to the end of the hole (without having to write any data).
> +        */
> +       if (out.seek_offset > 0 && (out.flags & ISTRUNC)) {
> +               if (ftruncate(out.fd, out.seek_offset) == -1)
> +                       err(1, "truncating %s", out.name);
> +       }
>  }
>
>  void
> @@ -457,29 +465,27 @@ dd_out(int force)
>                         }
>                         if (sparse && !force) {
>                                 pending += cnt;
> -                               last_sp = cnt;
>                                 nw = cnt;
>                         } else {
>                                 if (pending != 0) {
> -                                       /* If forced to write, and we have
> no
> -                                        * data left, we need to write the
> last
> -                                        * sparse block explicitly.
> +                                       /*
> +                                        * Seek past hole.  Note that we
> need to record the
> +                                        * reached offset, because we
> might have no more data
> +                                        * to write, in which case we'll
> need to call
> +                                        * ftruncate to extend the file
> size.
>                                          */
> -                                       if (force && cnt == 0) {
> -                                               pending -= last_sp;
> -                                               assert(outp == out.db);
> -                                               memset(outp, 0, cnt);
> -                                       }
> -                                       if (lseek(out.fd, pending,
> SEEK_CUR) ==
> -                                           -1)
> +                                       out.seek_offset = lseek(out.fd,
> pending, SEEK_CUR);
> +                                       if (out.seek_offset == -1)
>                                                 err(2, "%s: seek error
> creating sparse file",
>                                                     out.name);
> -                                       pending = last_sp = 0;
> +                                       pending = 0;
>                                 }
> -                               if (cnt)
> +                               if (cnt) {
>                                         nw = write(out.fd, outp, cnt);
> -                               else
> +                                       out.seek_offset = 0;
> +                               } else {
>                                         return;
> +                               }
>                         }
>
>                         if (nw <= 0) {
>
> Modified: head/bin/dd/dd.h
>
> ==============================================================================
> --- head/bin/dd/dd.h    Thu Feb 18 07:44:14 2016        (r295748)
> +++ head/bin/dd/dd.h    Thu Feb 18 08:44:16 2016        (r295749)
> @@ -54,6 +54,7 @@ typedef struct {
>         const char      *name;          /* name */
>         int             fd;             /* file descriptor */
>         off_t           offset;         /* # of blocks to skip */
> +       off_t           seek_offset;    /* offset of last seek past output
> hole */
>  } IO;
>
>  typedef struct {
>
> Modified: head/bin/dd/gen.c
>
> ==============================================================================
> --- head/bin/dd/gen.c   Thu Feb 18 07:44:14 2016        (r295748)
> +++ head/bin/dd/gen.c   Thu Feb 18 08:44:16 2016        (r295749)
> @@ -5,13 +5,20 @@
>   */
>
>  #include <stdio.h>
> +#include <string.h>
>
>  int
> -main(int argc __unused, char **argv __unused)
> +main(int argc, char **argv)
>  {
>         int i;
>
> -       for (i = 0; i < 256; i++)
> -               putchar(i);
> +       if (argc > 1 && !strcmp(argv[1], "189284")) {
> +               fputs("ABCDEFGH", stdout);
> +               for (i = 0; i < 8; i++)
> +                       putchar(0);
> +       } else {
> +               for (i = 0; i < 256; i++)
> +                       putchar(i);
> +       }
>         return (0);
>  }
>
> Added: head/bin/dd/ref.obs_zeroes
>
> ==============================================================================
> --- /dev/null   00:00:00 1970   (empty, because file is newly added)
> +++ head/bin/dd/ref.obs_zeroes  Thu Feb 18 08:44:16 2016        (r295749)
> @@ -0,0 +1,3 @@
> +$FreeBSD$
> +00000000  41 42 43 44 45 46 47 48  00 00 00 00 00 00 00 00
> |ABCDEFGH........|
> +00000010
>
>


More information about the svn-src-all mailing list