Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Feb 2016 21:38:23 +0100
From:      Thomas Quinot <thomas@FreeBSD.ORG>
To:        Maxim Sobolev <sobomax@FreeBSD.org>
Cc:        kib@freebsd.org, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn: head/bin/dd
Message-ID:  <20160210203823.GA74615@melamine.cuivre.fr.eu.org>
In-Reply-To: <CAH7qZfs=1vebh1jJVF2Gz1VViAaMNTip0p_oKq7Sb4yAHefwJw@mail.gmail.com>
References:  <201405071933.s47JXUx0046697@svn.freebsd.org> <CAH7qZfs=1vebh1jJVF2Gz1VViAaMNTip0p_oKq7Sb4yAHefwJw@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--XsQoSWH+UP9D9v3l
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

* 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:
>=20
> +                                       if (force && cnt =3D=3D 0) {
> +                                               pending -=3D last_sp;
> +                                               assert(outp =3D=3D out.db=
);
> +                                               memset(outp, 0, cnt);
> +                                       }
>=20
> 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 !=3D 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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- dd.c	(r=E9vision 265593)
+++ dd.c	(copie de travail)
@@ -470,6 +470,7 @@
 					 */
 					if (force && cnt =3D=3D 0) {
 						pending -=3D last_sp;
+						cnt +=3D last_sp;
 						assert(outp =3D=3D 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:
>=20
> http://sobomax.sippysoft.com/dd.diff
>=20
> Please fix.

Leveraging ftruncate certainly seems attractive, if the destination is a
regular file. I'll look into this option.

Thanks!
Thomas.


--XsQoSWH+UP9D9v3l
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQJ8BAEBCgBmBQJWu5+zXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRDRUU5NDNCMDA4OUI3QTg3NUZGMDg3RDdE
RjhFMEI1QzdDQzYyRUYyAAoJEN+OC1x8xi7ySXAP/j2bfwtylmhCJ0I9b8oqQ1Yx
+yxxhXFc8ovcfUFwoze1W70ijps5bpMwebGnvMBj7HPqd/08dgquG95JTlhKLThg
1BhrOV0rz2kAGd2W6ewXxeUDSLl7SQ69jI6wKcmkNu8wFEkAdE2ZkYQiaz5HtS8V
lhPkbdwjIeB8bzLvhyVWaBx0VXs0GpoIvf184PhbUb79uMkWk0xcC863JGI0S29g
RhFGOF8NFnfXNH1+nV6S9nfcYPmdq6qqCLLwzghQSSHZfCAe+aSlDm86wmK2B3XB
g0qbkKhTMQ3boFiM+JYcpbdVh1JTuynqWLz2qAxwQ4l0FyPknDwboI/AaDbFh5Jf
M3OseI183IJqTztdDIs/LhOZnc6nJhGvNb4jEsx+pt1J9O+rQR72phEr5ObxBqOf
ZiXrhpLR6D2zpELusWuTAJ2nJw3kPPOAgb979Nhl0jt4WpelumPvbOyR9UUBFZmW
d3eROWUWXsf3En6C1G9Puwtq5jafUecnDaTK+5c/txM1iDxdie2+3IQWF0oHy/eQ
tb/AF+Rrl6/BO/5Rc8sEJ1ArzP/g8hRkaVEvL5jYlLWjb+1KFSkcrPWQNO+l4vGZ
i07Li7tSGheHj33P2Qwsii2QFOtm+oS9GyUokh+DbJvEcrbaaSymAy+Q0a8wRP/Q
NffgaysChYiFB9IxU2Uv
=zgdt
-----END PGP SIGNATURE-----

--XsQoSWH+UP9D9v3l--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160210203823.GA74615>