From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#24340: insert-file-contents calls before-change-functions too late Date: Tue, 30 Aug 2016 14:42:19 -0400 Message-ID: NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1472582422 14600 195.159.176.226 (30 Aug 2016 18:40:22 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 30 Aug 2016 18:40:22 +0000 (UTC) To: 24340@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Aug 30 20:40:18 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1benxJ-0003NK-38 for geb-bug-gnu-emacs@m.gmane.org; Tue, 30 Aug 2016 20:40:17 +0200 Original-Received: from localhost ([::1]:50740 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benxG-00010m-Nf for geb-bug-gnu-emacs@m.gmane.org; Tue, 30 Aug 2016 14:40:14 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:43460) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benx9-0000xy-Iq for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:40:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1benx4-0001sl-B0 for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:40:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:46962) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benx4-0001sh-7d for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:40:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1benx3-0007dP-VZ for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:40:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 30 Aug 2016 18:40:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 24340 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.147258237229302 (code B ref -1); Tue, 30 Aug 2016 18:40:01 +0000 Original-Received: (at submit) by debbugs.gnu.org; 30 Aug 2016 18:39:32 +0000 Original-Received: from localhost ([127.0.0.1]:44674 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1benwa-0007cY-9T for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:32 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:52412) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1benwZ-0007cL-23 for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:31 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1benwS-0001lC-SX for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:26 -0400 Original-Received: from lists.gnu.org ([2001:4830:134:3::11]:58445) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benwS-0001kr-P5 for submit@debbugs.gnu.org; Tue, 30 Aug 2016 14:39:24 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:43348) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benwQ-0000mk-BK for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:39:23 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1benwL-0001io-88 for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:39:21 -0400 Original-Received: from chene.dit.umontreal.ca ([132.204.246.20]:51594) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1benwL-0001ia-3D for bug-gnu-emacs@gnu.org; Tue, 30 Aug 2016 14:39:17 -0400 Original-Received: from ceviche.home (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.7/8.14.1) with ESMTP id u7UIdtsT003418 for ; Tue, 30 Aug 2016 14:39:55 -0400 Original-Received: by ceviche.home (Postfix, from userid 20848) id 7F8A966274; Tue, 30 Aug 2016 14:42:19 -0400 (EDT) X-NAI-Spam-Flag: NO X-NAI-Spam-Level: X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 0.6 X-NAI-Spam-Rules: 4 Rules triggered BEC_TRC1=0.2, BEC_TRC1_W_GEN_SPAM_FEATRE=0.2, GEN_SPAM_FEATRE=0.2, RV5782=0 X-NAI-Spam-Version: 2.3.0.9418 : core <5782> : inlines <5157> : streams <1692582> : uri <2278217> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x 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: 208.118.235.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:122781 Archived-At: Package: Emacs Version: 25.1.50 Recipe: emacs -Q lisp/subr.el -f auto-revert-mode-hook M-: (defun sm-foo (&rest args) (message "b-c-f: %S size=%S" args (buffer-size))) RET M-: (add-hook 'before-change-functions #'sm-foo nil t) RET then zile lisp/subr.el finally check *Messages*: you should see that the first message will say something like "b-c-f: (...) NNN" where NNN is *smaller* than the original buffer size. IOW b-c-f is called after some part of the buffer has already been removed. Indeed, the bug is in Finsert_file_contents where we do: if (same_at_end != same_at_start) { invalidate_buffer_caches (current_buffer, BYTE_TO_CHAR (same_at_start), same_at_end_charpos); del_range_byte (same_at_start, same_at_end, 0); [...] /* This binding is to avoid ask-user-about-supersession-threat being called in insert_from_buffer (via in prepare_to_modify_buffer). */ specbind (intern ("buffer-file-name"), Qnil); insert_from_buffer (XBUFFER (conversion_buffer), same_at_start_charpos, inserted_chars, 0); The call to del_range_byte has arg prepare=0 so it doesn't run b-c-f. Instead b-c-f is called from insert_from_buffer, which is too late since the deletion already took place. This is also the source of the known bug that b-c-f is not called at all if insert-file-contents ends up only deleting text (since in that case del_range_byte is called but insert_from_buffer isn't). I include a suggested patch (which also would have the consequence that we could get rid of the `prepare' argument of del_range_byte). Stefan diff --git a/src/fileio.c b/src/fileio.c index bf6bf62..55331d9 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -3839,6 +3839,7 @@ by calling `format-decode', which see. */) if (! giveup_match_end) { ptrdiff_t temp; + ptrdiff_t this_count = SPECPDL_INDEX (); /* We win! We can handle REPLACE the optimized way. */ @@ -3868,13 +3869,15 @@ by calling `format-decode', which see. */) beg_offset += same_at_start - BEGV_BYTE; end_offset -= ZV_BYTE - same_at_end; + specbind (intern ("buffer-file-name"), Qnil); invalidate_buffer_caches (current_buffer, BYTE_TO_CHAR (same_at_start), same_at_end_charpos); - del_range_byte (same_at_start, same_at_end, 0); + del_range_byte (same_at_start, same_at_end, true); /* Insert from the file at the proper position. */ temp = BYTE_TO_CHAR (same_at_start); SET_PT_BOTH (temp, same_at_start); + unbind_to (this_count, Qnil); /* If display currently starts at beginning of line, keep it that way. */ @@ -3979,10 +3982,11 @@ by calling `format-decode', which see. */) /* Truncate the buffer to the size of the file. */ if (same_at_start != same_at_end) { + specbind (intern ("buffer-file-name"), Qnil); invalidate_buffer_caches (current_buffer, BYTE_TO_CHAR (same_at_start), BYTE_TO_CHAR (same_at_end)); - del_range_byte (same_at_start, same_at_end, 0); + del_range_byte (same_at_start, same_at_end, true); } inserted = 0; @@ -4030,12 +4034,23 @@ by calling `format-decode', which see. */) we are taking from the decoded string. */ inserted -= (ZV_BYTE - same_at_end) + (same_at_start - BEGV_BYTE); + /* This binding is to avoid ask-user-about-supersession-threat + being called in insert_from_buffer or del_range_bytes (via in + prepare_to_modify_buffer). + AFAICT this is mostly needed because we change the buffer + before we set current_buffer->modtime, but if the file is modified + while we read from it, even if we set current_buffer->modtime first we + could still end up calling ask-user-about-supersession-threat + which wouldn't make much sense. */ + specbind (intern ("buffer-file-name"), Qnil); if (same_at_end != same_at_start) { + /* FIXME: Shouldn't invalidate_buffer_caches be called by + del_range_byte or even directly by prepare_to_modify_buffer? */ invalidate_buffer_caches (current_buffer, BYTE_TO_CHAR (same_at_start), same_at_end_charpos); - del_range_byte (same_at_start, same_at_end, 0); + del_range_byte (same_at_start, same_at_end, true); temp = GPT; eassert (same_at_start == GPT_BYTE); same_at_start = GPT_BYTE; @@ -4056,10 +4071,6 @@ by calling `format-decode', which see. */) same_at_start + inserted - BEGV_BYTE + BUF_BEG_BYTE (XBUFFER (conversion_buffer))) - same_at_start_charpos); - /* This binding is to avoid ask-user-about-supersession-threat - being called in insert_from_buffer (via in - prepare_to_modify_buffer). */ - specbind (intern ("buffer-file-name"), Qnil); insert_from_buffer (XBUFFER (conversion_buffer), same_at_start_charpos, inserted_chars, 0); /* Set `inserted' to the number of inserted characters. */