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 v3] Don't rewrite buffer contents after saving by rename Date: Wed, 1 May 2019 16:02:01 -0700 Message-ID: <20190501230200.46879-1-jktomer@google.com> References: <83tveeosqe.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="201231"; mail-complaints-to="usenet@blaine.gmane.org" Cc: Jonathan Tomer To: 35497@debbugs.gnu.org, eliz@gnu.org, michael.albinus@gmx.de Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu May 02 01:04:26 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 1hLyH3-000qCA-IH for geb-bug-gnu-emacs@m.gmane.org; Thu, 02 May 2019 01:04:25 +0200 Original-Received: from localhost ([127.0.0.1]:42519 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hLyH2-0001er-KH for geb-bug-gnu-emacs@m.gmane.org; Wed, 01 May 2019 19:04:24 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:36283) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hLyGw-0001em-Bo for bug-gnu-emacs@gnu.org; Wed, 01 May 2019 19:04:19 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hLyGo-0003O2-1H for bug-gnu-emacs@gnu.org; Wed, 01 May 2019 19:04:13 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:59863) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hLyGf-0003HN-Tx for bug-gnu-emacs@gnu.org; Wed, 01 May 2019 19:04:05 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hLyGf-0003x0-K3 for bug-gnu-emacs@gnu.org; Wed, 01 May 2019 19:04:01 -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 23:04:01 +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.155675180115140 (code B ref 35497); Wed, 01 May 2019 23:04:01 +0000 Original-Received: (at 35497) by debbugs.gnu.org; 1 May 2019 23:03:21 +0000 Original-Received: from localhost ([127.0.0.1]:45173 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hLyG1-0003w8-Fn for submit@debbugs.gnu.org; Wed, 01 May 2019 19:03:21 -0400 Original-Received: from mail-pf1-f202.google.com ([209.85.210.202]:35653) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <3sSXKXAcKBRI12B64w9y66y3w.u64LNMRPvwttCyA.y5C.69y@flex--jktomer.bounces.google.com>) id 1hLyG0-0003vu-36 for 35497@debbugs.gnu.org; Wed, 01 May 2019 19:03:20 -0400 Original-Received: by mail-pf1-f202.google.com with SMTP id c12so215728pfb.2 for <35497@debbugs.gnu.org>; Wed, 01 May 2019 16:03:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=aVIyvkjVbA4nZTutMPnEMIqUhRpz8yoqfPi+yWC8FeE=; b=NCMv+puEoB6D6emJNwR1mV1kclEDnbJoGk9sPWLj2frOBQPeaxVll1mxk+HX3Hqg6E ftw+jjDzFDQWgxub/DS53WPIkpvAVZFFvqLf2Noof0DyzQESw3pcKNs3nqSokwoQAi6A +zKY7q8fmMyH/TWQA9YNvMWE76Z8KZZbv3CFIhTAzNew9+CUQNZi2SfTM8zvy7J3qlOO nxNJwXnDU6w1eiiixUebLiQcs4+UM8PA0Zg6mQylw0wPzNtkeHCxRa8pmTU7GZ86FGzL 1x09gQmmMHWAw9iMiqvezXYIOK/Z+DC6bak5Zb3kDD3G5hZgPqo1QO0kgc7km2S7QP0L YBIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=aVIyvkjVbA4nZTutMPnEMIqUhRpz8yoqfPi+yWC8FeE=; b=koNxin5g6uvkNMxnwJWMOghYYcYVxpJZNqGbxLwI5CWdpLcN6HWNlSGc6Z7oS5pNfe YUL+Ir8UTsEJURUNirz3m9cLvyO9/Zb0oSFOYsH3XS0A3KTJZqRBq/9EqOoma3Nzpb6P zsbeL2yN6+WTqe/3H2r8o9Nq7JIY+6U6IknqvparqCUGLE1XgnENkPH/nHPqo3X9oqpj eAaFDSxulsf+qXJW6Y5Q3Yq9a+iB60l6z8/FMXE+8NHLfQ+n2+ijImli9H16EtM5ZPPm vhDBmEgI3ZSsUPkfiqH+JLko4dsJDSHh4/3bZ04SCFhddTvkOFDdQowX0goJ95axA9cv V4pQ== X-Gm-Message-State: APjAAAWILCMxiZBs8RYuQcgh0+UKQjrf/RthNQ5qB1RNqhCvjJ8WZwi5 tJKZNbECPYdVPDEH6uQbHZwVUf4+YCfNqnbHH4bVO8vK0MFtiZPmTAAuLYH5CqJh/bP5f8pA3YM h7UYaZ9J/Y9NXrPFJ7eoY8nLJ2gqknfXXvfQI3DeZgThEpIiI1k67hPUv31jF32I= X-Google-Smtp-Source: APXvYqxAZK4ZY+8izXaFDmCJqEsGo1Yr249IapzezS/01AObMnv5nTzrQ8qlFsFnKcwGY89HZP5mZZS1HG2B X-Received: by 2002:a65:60d7:: with SMTP id r23mr531064pgv.223.1556751793511; Wed, 01 May 2019 16:03:13 -0700 (PDT) In-Reply-To: <83tveeosqe.fsf@gnu.org> X-Mailer: git-send-email 2.21.0.593.g511ec345e18-goog 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:158612 Archived-At: When `file-precious-flag' is non-nil, files are saved by renaming a temporary file to the new name; this is an atomic operation on POSIX so other programs will not see the file in an intermediate state. Unfortunately, due to a paren-matching error introduced in change 574c05e219476912db3105fa164accd9ba12b35f, we would then write the contents again in the usual way after this rename. In addition to being wasteful, this is a serious bug: the whole point of `file-precious-flag' is to prevent race conditions with other programs that might otherwise see an empty file, but with this bug the race is actually much *more* likely to be visible: the rename will alert any inotify watchers of a change, and then the subsequent write is very likely to truncate the file just as those programs start to read it! * lisp/files.el (basic-save-buffer-2): Don't rewrite file contents after saving-by-renaming. * test/lisp/files-tests.el (files-tests-dont-rewrite-precious-files): * test/lisp/net/tramp-tests.el (tramp-test46-file-precious-flag): Regression tests for this change. --- lisp/files.el | 4 ++-- test/lisp/files-tests.el | 13 +++++++++++++ test/lisp/net/tramp-tests.el | 16 ++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lisp/files.el b/lisp/files.el index c05d70a00e..72518e8127 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -5256,7 +5256,7 @@ basic-save-buffer-2 (set-file-extended-attributes buffer-file-name (nth 1 setmodes))) (set-file-modes buffer-file-name - (logior (car setmodes) 128)))))) + (logior (car setmodes) 128))))) (let (success) (unwind-protect (progn @@ -5272,7 +5272,7 @@ basic-save-buffer-2 (and setmodes (not success) (progn (rename-file (nth 2 setmodes) buffer-file-name t) - (setq buffer-backed-up nil)))))) + (setq buffer-backed-up nil))))))) setmodes)) (declare-function diff-no-select "diff" diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el index ae8ea41a79..0becde4184 100644 --- a/test/lisp/files-tests.el +++ b/test/lisp/files-tests.el @@ -1244,5 +1244,18 @@ files-tests-file-attributes-equal (executable-find (file-name-nondirectory tmpfile)))))) (delete-file tmpfile)))) +(ert-deftest files-tests-dont-rewrite-precious-files () + "Test that `file-precious-flag' forces files to be saved by +renaming only, rather than modified in-place." + (files-tests--with-temp-file temp-file-name + (files-tests--with-advice + write-region :before + (lambda (_start _end filename &rest r) + (should (not (string= filename temp-file-name)))) + (with-current-buffer (find-file-noselect temp-file-name) + (setq-local file-precious-flag t) + (insert "foobar") + (should (null (save-buffer))))))) + (provide 'files-tests) ;;; files-tests.el ends here diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el index cba697da18..03ce6a5e94 100644 --- a/test/lisp/net/tramp-tests.el +++ b/test/lisp/net/tramp-tests.el @@ -5741,6 +5741,22 @@ tramp--test-asynchronous-requests-timeout (ignore-errors (all-completions "tramp" (symbol-value x))) (ert-fail (format "Hook `%s' still contains Tramp function" x)))))) +(ert-deftest tramp-test46-file-precious-flag () + "Check that file-precious-flag is respected with Tramp in use." + (let* ((temp-file (make-temp-file "emacs")) + (remote-file (concat "/mock:localhost:" temp-file)) + (advice (lambda (_start _end filename &rest r) + (should (not (string= filename temp-file))) + (should (not (string= filename remote-file)))))) + (unwind-protect + (with-current-buffer (find-file-noselect remote-file) + (advice-add 'write-region :before advice) + (setq-local file-precious-flag t) + (insert "foobar") + (should (null (save-buffer)))) + (advice-remove 'write-region advice) + (delete-file temp-file)))) + (defun tramp-test-all (&optional interactive) "Run all tests for \\[tramp]." (interactive "p") -- 2.21.0.593.g511ec345e18-goog