bug#35497: [PATCH] Don't rewrite buffer contents after saving by rename

[Jonathan Tomer <jktomer@google.com>]

[-- Attachment #1: Type: text/plain, Size: 2561 bytes --]

On Wed, May 1, 2019, 10:48 Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Jonathan Tomer <jktomer@google.com>
> > 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.

>

[-- Attachment #2: Type: text/html, Size: 3466 bytes --]