svn: head/bin/dd

[Maxim Sobolev]

The patch looks fair to me, although I have not got chance to test it yet.
You probably don't need += since cnt is already tested to be 0, so "="
would suffice. :)

In any case, I've opened a bug here:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=207092

So please make sure to deal with it when you are done with the commit. On
another note it would be nice to add some kind of basic test case for the
dd to verify correct behavior while we at it.

Thanks!

On Wed, Feb 10, 2016 at 12:38 PM, Thomas Quinot <thomas at freebsd.org> wrote:

> * Maxim Sobolev, 2016-02-10 :
>
> > 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.
>
> Correct :-(
>
> In the context of the current logic, I think the simple fix is:
>
> Index: dd.c
> ===================================================================
> --- dd.c        (révision 265593)
> +++ dd.c        (copie de travail)
> @@ -470,6 +470,7 @@
>                                          */
>                                         if (force && cnt == 0) {
>                                                 pending -= last_sp;
> +                                               cnt += last_sp;
>                                                 assert(outp == out.db);
>                                                 memset(outp, 0, cnt);
>                                         }
>
> > 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
> >
> > Please fix.
>
> Leveraging ftruncate certainly seems attractive, if the destination is a
> regular file. I'll look into this option.
>
> Thanks!
> Thomas.
>
>