From owner-svn-src-head@freebsd.org Wed Feb 10 21:29:49 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6B626AA47C0 for ; Wed, 10 Feb 2016 21:29:49 +0000 (UTC) (envelope-from sobomax@sippysoft.com) Received: from mail-wm0-x22c.google.com (mail-wm0-x22c.google.com [IPv6:2a00:1450:400c:c09::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id EBD5E365 for ; Wed, 10 Feb 2016 21:29:48 +0000 (UTC) (envelope-from sobomax@sippysoft.com) Received: by mail-wm0-x22c.google.com with SMTP id c200so48036053wme.0 for ; Wed, 10 Feb 2016 13:29:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sippysoft-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=RmoszIHyOiOeC9hDGBBNkjzJQMi56+Jr00tYE/9Zlws=; b=XxSpi8/YkScJbrXUrRwqhkgdUlmwE6aClc3+r/sm0jBwwfeR1OGEB+rcBqoEDuEXw7 IKIwzZ/vSMyhCAgDgGua+T44de4sKQcaeXyriOlUSMjhqGH5uA4UizrwJmMRoedQfCf8 WesPl/IFgw0HXoWsqvudWFu7a4CmE//l6rAvQWxpX6yqvyclUPTbYr9Qr5fOfpW3U6xz v633Qwf6dWHHnkHhtfsvYplwhpHvdO+Eyyx2FD/k1nf4Dd0ceJc7zLgVwiaN/x2H9GZG Ug+LQrcMiFf/KA5UrZvUHNrtm5u/JtPyGuSWjjYw4FlEtfcLhc1ZAMzA6tvwwmgbf7kS GN9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=RmoszIHyOiOeC9hDGBBNkjzJQMi56+Jr00tYE/9Zlws=; b=X+F2KqvEdCiyXHe17MH7ZMVpNSX1+psbzCysUeptkuOAJ10VhofRdnZoqVmknTsCNF ybBFJWcssclZ+c29zK9QhFgB4Q7P/EeVxV+23M3Fka5zLL5Pm/zKJbQSpYTaYSImIdvW I4NOEV6eAxk+nbk0yteI7DtMOkxhtSLkC5ZO2/FwPPjXL0NylV9tuo5uXrnEk7XWNYVT qLNfiQb8mNzvJMuBnaGA9qS5T6IlO7BmTetS8gTxSl6Qn9H9+lr866kSRqLXoaZYTZl0 eP2tFJVAIEvdVKxJuGvo9VGIcRVQTafqIHEuECijrZZ3wrVbWTyX+G83DEksj/RMQJ1G 1+EA== X-Gm-Message-State: AG10YOTEavSaNGXCP1Z55mVOoixcFtkWGd0pkKSwpNpJzwnXsLzQ7t6BAjcrS2GzMr31J9MetnsztCye3HPkIJLW MIME-Version: 1.0 X-Received: by 10.28.225.8 with SMTP id y8mr14498095wmg.94.1455139787439; Wed, 10 Feb 2016 13:29:47 -0800 (PST) Sender: sobomax@sippysoft.com Received: by 10.27.172.16 with HTTP; Wed, 10 Feb 2016 13:29:47 -0800 (PST) In-Reply-To: <20160210203823.GA74615@melamine.cuivre.fr.eu.org> References: <201405071933.s47JXUx0046697@svn.freebsd.org> <20160210203823.GA74615@melamine.cuivre.fr.eu.org> Date: Wed, 10 Feb 2016 13:29:47 -0800 X-Google-Sender-Auth: _vfkvHRIUbv0TEz5rEFJ8P3edVs Message-ID: Subject: Re: svn: head/bin/dd From: Maxim Sobolev To: Thomas Quinot Cc: kib@freebsd.org, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.20 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Feb 2016 21:29:49 -0000 The patch looks fair to me, although I have not got chance to test it yet. You probably don't need +=3D since cnt is already tested to be 0, so "=3D" would suffice. :) In any case, I've opened a bug here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D207092 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 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 =3D=3D 0) { > > + pending -=3D last_sp; > > + assert(outp =3D=3D 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 !=3D 0, so that it's nev= er > > 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=C3=A9vision 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 seem= s > 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. > >