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: Wed, 1 May 2019 12:29:42 -0700 Message-ID: References: <20190429232009.94549-1-jktomer@google.com> <87pnp4t0zp.fsf@gmx.de> <875zqvz0co.fsf@gmx.de> <8336lyqd52.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="000000000000cc699c0587d88a9d" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="88090"; mail-complaints-to="usenet@blaine.gmane.org" Cc: 35497@debbugs.gnu.org, Michael Albinus To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed May 01 21:31:13 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 1hLuwj-000MiV-4q for geb-bug-gnu-emacs@m.gmane.org; Wed, 01 May 2019 21:31:13 +0200 Original-Received: from localhost ([127.0.0.1]:40121 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hLuwi-0002ZR-2b for geb-bug-gnu-emacs@m.gmane.org; Wed, 01 May 2019 15:31:12 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:40590) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hLuwb-0002XI-D7 for bug-gnu-emacs@gnu.org; Wed, 01 May 2019 15:31:06 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hLuwZ-0000Xj-Bk for bug-gnu-emacs@gnu.org; Wed, 01 May 2019 15:31:05 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:59648) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hLuwY-0000Wu-DE for bug-gnu-emacs@gnu.org; Wed, 01 May 2019 15:31:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hLuwY-0005Iz-73 for bug-gnu-emacs@gnu.org; Wed, 01 May 2019 15:31: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: Wed, 01 May 2019 19:31: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.155673900520258 (code B ref 35497); Wed, 01 May 2019 19:31:02 +0000 Original-Received: (at 35497) by debbugs.gnu.org; 1 May 2019 19:30:05 +0000 Original-Received: from localhost ([127.0.0.1]:44959 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hLuvc-0005Gf-2q for submit@debbugs.gnu.org; Wed, 01 May 2019 15:30:04 -0400 Original-Received: from mail-ua1-f68.google.com ([209.85.222.68]:32921) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hLuvZ-0005Ff-Rt for 35497@debbugs.gnu.org; Wed, 01 May 2019 15:30:03 -0400 Original-Received: by mail-ua1-f68.google.com with SMTP id x6so3565306uaq.0 for <35497@debbugs.gnu.org>; Wed, 01 May 2019 12:30:01 -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=d1ZmgSbT52gHmhuaJVJSC8wTcylervSTnpLPHOteAyc=; b=g7QOC62kZGlWUqDWIAi8EP+tIDFDilCCOlrULCneWmsdIPq71DvYkwyMQCAPHf3HER xIUsRMF4tYIGvMWF9ExOsQ1/Vhb/ZvAgBufaVlpoDeAofBaggud1Oe2lthQh+syYIPxo F9NxZNloOnweThvBYy0yFNGSdISX6kliE2juKC8KJc+eGPdmJ6Iy2Z7DNqnqxN2wGx6x R95y0nNnx500Gq7j9ql96Fqq6sJ43bSMs99xmW+0tI6eO1jFCQonhXwyKZDp2Ze9YS9b w0G+aCj5LYKKBSZrWAvotgSiLStd1WH6DWg2GR40rn0RzEW0Qm4hu8siwwAsJZuDHq/C 3oYg== 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=d1ZmgSbT52gHmhuaJVJSC8wTcylervSTnpLPHOteAyc=; b=VeuWqaMOwsyB1lU+ovHShTvUtEiN0dQtxaGoLWYtlRgsZ2Kgf3HRKMh4MwK4YdEX90 Xk3tyjm+AnJUfjqfUd0luneYUzE1TN8Fzt0cScc9Ll7HtbqUq637amSd2MKsM5GK1Fy/ qA/SgB+vLI3Dc2hS8AeiKVP38vZvJX/orElg1YofWyIIbP2rAgTJMmhsqhIO0Q7syLTT iaW/9M4OLlLdDRm1xHkLEIGnIBo9OFaK5EQ87XghQbiJMU73O1ujlOgXVjuvp5lsEZEV Y2Jy8Kfnz/PcLkXwFLgM2Av8WSujkfntBkHe4qxR8QSAP59c2+nzgA/ckmezzY92qzD6 MN7g== X-Gm-Message-State: APjAAAVkaWpG92J+xxYiMjQ/N9e3bNf7D0blRvczuOCk3/aqglYQ66uu UiKvyu5dFkAZAP2PFRrRKR1GjgBE1eNEO12yVAzj6A== X-Google-Smtp-Source: APXvYqzVG9xuwRPQze2nXYnHp6tNR4jO98Ot9I6OpfV4+estIbwX/8v/fMd8R2d7xTsqMtXHRyHUWEgjZUl6m6NA4QE= X-Received: by 2002:ab0:2a13:: with SMTP id o19mr12067646uar.132.1556738994862; Wed, 01 May 2019 12:29:54 -0700 (PDT) In-Reply-To: <8336lyqd52.fsf@gnu.org> 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:158598 Archived-At: --000000000000cc699c0587d88a9d Content-Type: text/plain; charset="UTF-8" On Wed, May 1, 2019, 10:48 Eli Zaretskii wrote: > > From: Jonathan Tomer > > Date: Tue, 30 Apr 2019 14:10:32 -0700 > > Cc: 35497@debbugs.gnu.org > > > > 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.) > > I suggest to make a step back and clearly define what you are trying > to test. Is it that we don't rewrite the file after saving it, or is > it some condition regarding the inodes of the original and the new > file? > > These are two different things, and the second one is extremely > platform-dependent (because inodes exist only in certain filesystems, > and are emulated in others, and also because which notifications are > generated in such complex situations is very hard to guess in > advance). > Indeed. What I am testing, as you say, is that the file is not changed by writing to it under its final name (ever, not just after renaming, though the latter happened to be the bug in this case) when file-precious-flag is non-nil. Any discussion of inodes was only because of the perceived unreliability, and actual relative unportability, of filenotify and the systems underlying it. It's true that notifications are imperfect, but IMO they are the only possible way to test that particular invariant, and this test implementation is designed to be as strict as the available notification system allows without breaking under any reasonable notification API. > --000000000000cc699c0587d88a9d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Wed, May 1, 2019, 10:48 Eli Zaretskii <eliz@gnu.org> wrote:
> From: Jonathan Tomer <jktomer@google.com><= br> > Date: Tue, 30 Apr 2019 14:10:32 -0700
> Cc: 35497@debbugs.gnu.org
>
>=C2=A0 Btw, your new test in files-tests.el uses file notifications. Po= ssible
>=C2=A0 of course. But wouldn't it be more robust to check the inode= number of
>=C2=A0 the involved files, and how it changes, or not? See file-attribu= tes how
>=C2=A0 to retrieve the inode number of a file.
>
> I thought about checking that the inode number changes, but that would= n'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 doe= sn't
> appear to be easily caught with any examination of the final state alo= ne,
> since what we're looking for is to prove the *absence* of any writ= e that fails
> to change the inode number. (Perhaps we could check that the modificat= ion time
> of the file, after write, is *less* than its inode change time, provin= g that
> there has been no ordinary write since the rename -- but in my experie= nce,
> inode timestamps are not actually more reliable than inotify, and in > particular this check is easily defeated by the mode-setting that happ= ens
> after the write is complete, requiring care to make sure that save-buf= fer will
> not attempt to do so.)

I suggest to make a step back and clearly define what you are trying
to test.=C2=A0 Is it that we don't rewrite the file after saving it, or= is
it some condition regarding the inodes of the original and the new
file?

These are two different things, and the second one is extremely
platform-dependent (because inodes exist only in certain filesystems,
and are emulated in others, and also because which notifications are
generated in such complex situations is very hard to guess in
advance).

Indeed. What I am testing, as you say, is that the file is not ch= anged by writing to it under its final name (ever, not just after renaming,= though the latter happened to be the bug in this case) when file-precious-= flag is non-nil.

Any dis= cussion of inodes was only because of the perceived unreliability, and actu= al relative unportability, of filenotify and the systems underlying it.

It's true that notifica= tions are imperfect, but IMO they are the only possible way to test that pa= rticular invariant, and this test implementation is designed to be as stric= t as the available notification system allows without breaking under any re= asonable notification API.
--000000000000cc699c0587d88a9d--