From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Jonathan Tomer Newsgroups: gmane.emacs.bugs Subject: bug#35497: [PATCH] Don't rewrite buffer contents after saving by rename Date: Tue, 30 Apr 2019 14:10:32 -0700 Message-ID: References: <20190429232009.94549-1-jktomer@google.com> <87pnp4t0zp.fsf@gmx.de> <875zqvz0co.fsf@gmx.de> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="0000000000007b7b970587c5d53a" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="180679"; mail-complaints-to="usenet@blaine.gmane.org" Cc: 35497@debbugs.gnu.org To: Michael Albinus Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Apr 30 23:11:17 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hLa21-000kqg-94 for geb-bug-gnu-emacs@m.gmane.org; Tue, 30 Apr 2019 23:11:17 +0200 Original-Received: from localhost ([127.0.0.1]:53566 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hLa1z-0000mz-Fg for geb-bug-gnu-emacs@m.gmane.org; Tue, 30 Apr 2019 17:11:15 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:33180) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hLa1n-0000l6-Tv for bug-gnu-emacs@gnu.org; Tue, 30 Apr 2019 17:11:05 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hLa1m-0006MX-GF for bug-gnu-emacs@gnu.org; Tue, 30 Apr 2019 17:11:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:57259) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hLa1m-0006MS-AG for bug-gnu-emacs@gnu.org; Tue, 30 Apr 2019 17:11:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hLa1m-0004Pa-4v for bug-gnu-emacs@gnu.org; Tue, 30 Apr 2019 17:11:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Jonathan Tomer Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 30 Apr 2019 21:11:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 35497 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 35497-submit@debbugs.gnu.org id=B35497.155665865216942 (code B ref 35497); Tue, 30 Apr 2019 21:11:02 +0000 Original-Received: (at 35497) by debbugs.gnu.org; 30 Apr 2019 21:10:52 +0000 Original-Received: from localhost ([127.0.0.1]:42570 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hLa1c-0004PB-46 for submit@debbugs.gnu.org; Tue, 30 Apr 2019 17:10:52 -0400 Original-Received: from mail-vs1-f42.google.com ([209.85.217.42]:42425) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hLa1Z-0004Ow-Ge for 35497@debbugs.gnu.org; Tue, 30 Apr 2019 17:10:50 -0400 Original-Received: by mail-vs1-f42.google.com with SMTP id b74so5745611vsd.9 for <35497@debbugs.gnu.org>; Tue, 30 Apr 2019 14:10:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=r4nY4lUaYCR+/BIrAbQDL5XjbxvGufiHUIrGLGwBwqY=; b=FLSjxTeUegAXOaavssMgf6WolXznsyki5YjRG5SxEafI9rpBpqX5M4iiNo1DzKgrmY 3f5/yRYuBPaeIDZKdYrhR8oB4cqLM13Tri+l91KWfvw5n5rbvZgfA/M7cDfoUMKSyFbi g3+hL/qgOqJUv4Xotf+o2jYfuleS+DHFUfEqYxzNgHMfQrKot4eA0YX0oX6vynOfQnvW K+hmSN0SYsWqmZnmbot01ZRTJ1tRSxzgjUfRA4ciSHo7H6O33JAfCtoRzY7ghRtz2/uA xKZL9eBpVkqitu0amwU4li9FyTJWjgR7iKt2Nt1gsww8ie2RjRsaGGFJ036R5RFcZR04 2DIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=r4nY4lUaYCR+/BIrAbQDL5XjbxvGufiHUIrGLGwBwqY=; b=MSWq8yQz2t6euV2JDIKqZ1i7CtPPRPolaxbw+znbmePNCg9nawo23IzY8rmtdqPZA3 0ihGy+eN7C2hLBR0/BR0yBTmTndS2gcf7WtVrflxIlLX6mN0PfDrs9JR1hTr7pY7Dqqn NM0jd7H1XSz5R3ld+fM5gaYYSEzmmw41q1IGW9Sgmr60gvpZFGeJwNsMTqooEiF3GQUE v2QwBIf2IXKPuYR4uMqEIJmmsCGKlvP8Be336olYgbvCF7s80GMBq4tJKZbK8MsufY+w ZxYUBHwPj5OtjU1HK9uAdb+1izCzHrYc1W3kHJ5iLLwfaPMmMZ/GKngEzOSUxSdDgaeX SADA== X-Gm-Message-State: APjAAAXuLgzCZytjm9HktQt2NaG5RZK4mhIkYwyS3QR4/IlIKFEtw7yC RUF7FLlCKsLAZajHzdD6RiP7G07+NLG/WjGgVV+f/w== X-Google-Smtp-Source: APXvYqyAZ1FyWmb8auQGMVbqoAqi+/zZZLhCLOdcF72XqpvHZDOpr98jeXRaDqjQTfJ9nis/wwMKYXZ3Y21I8tHBsnI= X-Received: by 2002:a67:f709:: with SMTP id m9mr20580953vso.171.1556658643467; Tue, 30 Apr 2019 14:10:43 -0700 (PDT) In-Reply-To: <875zqvz0co.fsf@gmx.de> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:158539 Archived-At: --0000000000007b7b970587c5d53a Content-Type: text/plain; charset="UTF-8" On Tue, Apr 30, 2019 at 1:47 PM Michael Albinus wrote: > Jonathan Tomer writes: > > Hi Jonathan, > > > Thanks, I'll fix. What's the preferred mechanism for sending an > > updated patch -- send an entirely new patch (relative to upstream) on > > a new thread, or on this thread, or a delta to apply as an additional > > commit on top of my previous patch? > > Please reply to these messages, keeping 35497@debbugs.gnu.org in To: or > Cc:. > This archives your message in the bug tracker. > > Usually, we appreciate a new patch relative to upstream. But in this > case, not changing etc/NEWS, I believe it isn't necessary to send a new > patch until there are other changes you want to show. > OK, will send along with the patch adding the TRAMP test shortly. > That said, I'm happy to add a test to tramp-tests.el as well; at the > > very least, with the mock tramp method we should see that the > > destination file is renamed-to rather than overwritten as well. > > Pls do. > > Btw, your new test in files-tests.el uses file notifications. Possible > of course. But wouldn't it be more robust to check the inode number of > the involved files, and how it changes, or not? See file-attributes how > to retrieve the inode number of a file. > I thought about checking that the inode number changes, but that wouldn't have caught this particular bug (where the file is renamed into place with the correct contents, and then rewritten in place again); indeed, that doesn't appear to be easily caught with any examination of the final state alone, since what we're looking for is to prove the *absence* of any write that fails to change the inode number. (Perhaps we could check that the modification time of the file, after write, is *less* than its inode change time, proving that there has been no ordinary write since the rename -- but in my experience, inode timestamps are not actually more reliable than inotify, and in particular this check is easily defeated by the mode-setting that happens after the write is complete, requiring care to make sure that save-buffer will not attempt to do so.) Best, Jonathan --0000000000007b7b970587c5d53a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Tue,= Apr 30, 2019 at 1:47 PM Michael Albinus <michael.albinus@gmx.de> wrote:
Jonathan Tomer= <jktomer@google.com> writes:

Hi Jonathan,

> Thanks, I'll fix. What's the preferred mechanism for sending a= n
> updated patch -- send an entirely new patch (relative to upstream) on<= br> > a new thread, or on this thread, or a delta to apply as an additional<= br> > commit on top of my previous patch?

Please reply to these messages, keeping 35497@de= bbugs.gnu.org in To: or Cc:.
This archives your message in the bug tracker.

Usually, we appreciate a new patch relative to upstream. But in this
case, not changing etc/NEWS, I believe it isn't necessary to send a new=
patch until there are other changes you want to show.
=
OK, will send along with the patch adding the TRAMP test sho= rtly.=C2=A0

> That said, I'm happy to add a test to tramp-tests.el as well; at t= he
> very least, with the mock tramp method we should see that the
> destination file is renamed-to rather than overwritten as well.

Pls do.

Btw, your new test in files-tests.el uses file notifications. Possible
of course. But wouldn't it be more robust to check the inode number of<= br> the involved files, and how it changes, or not? See file-attributes how
to retrieve the inode number of a file.

I thought about checking that the inode number changes, bu= t that wouldn't have
caught this particular bug (where the fi= le is renamed into place with the
correct contents, and then rewr= itten in place again); indeed, that doesn't
appear to be easi= ly caught with any examination of the final state alone,
since wh= at we're looking for is to prove the *absence* of any write that fails<= /div>
to change the inode number. (Perhaps we could check that the modi= fication time
of the file, after write, is *less* than its inode = change time, proving that
there has been no ordinary write since = the rename -- but in my experience,
inode timestamps are not actu= ally more reliable than inotify, and in
particular this check is = easily defeated by the mode-setting that happens
after the write = is complete, requiring care to make sure that save-buffer will
no= t attempt to do so.)

Best,
Jonatha= n
--0000000000007b7b970587c5d53a--