GNU bug report logs - #39585
after-change-functions called with invalid positions in call-process

Previous Next

Package: emacs;

Reported by: Clément Pit-Claudel <cpitclaudel <at> gmail.com>

Date: Thu, 13 Feb 2020 03:57:02 UTC

Severity: normal

Fixed in version 27.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 39585 in the body.
You can then email your comments to 39585 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to acm <at> muc.de, bug-gnu-emacs <at> gnu.org:
bug#39585; Package emacs. (Thu, 13 Feb 2020 03:57:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Clément Pit-Claudel <cpitclaudel <at> gmail.com>:
New bug report received and forwarded. Copy sent to acm <at> muc.de, bug-gnu-emacs <at> gnu.org. (Thu, 13 Feb 2020 03:57:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
To: bug-gnu-emacs <bug-gnu-emacs <at> gnu.org>
Subject: after-change-functions called with invalid positions in call-process
Date: Wed, 12 Feb 2020 22:56:49 -0500
Hi all,

Recent changes in call-process have introduced surprising behavior in after-change-functions (and caused bugs in flycheck, e.g. https://github.com/flycheck/flycheck/issues/1677).  From the documentation, I expected that functions added to the after-change-functions hook would be called only with valid positions.  However, the following snippets shows that it's not the case:

    (defun ~/after-change (beg end len)
      (message "(after-change %S %S %S); (buffer %S %S %S)"
               beg end len (point-min) (point-max) (buffer-size)))

    (with-current-buffer (get-buffer-create "*tmp*")
      (make-variable-buffer-local 'after-change-functions)
      (add-hook 'after-change-functions #'~/after-change)
      (call-process "echo" nil t t "Hello"))

Running it repeatedly, this is what I observe:

    (after-change 7 13 0); (buffer 1 7 6)
    (after-change 13 19 0); (buffer 1 13 12)
    (after-change 19 25 0); (buffer 1 19 18)
    (after-change 25 31 0); (buffer 1 25 24)

Note how each time the after-change-functions hook is called with a region past the end of the buffer.  It's as if after-change-functions was in fact call right before the insertion, instead of after.

Previous versions of Emacs didn't call after-change-functions in this case; it seems that the new behavior was introduced by this commit:

    commit 224e8d146485ce178086549d41fa8359dcc0e03e
    Author: Alan Mackenzie <acm <at> muc.de>
    Date:   Wed Jan 22 19:50:30 2020 +0000

        Make call_process call signal_after_change.  This fixes bug #38691.

        Now, functions such as call-proess-region invoke after-change-functions
        correctly.

        * src/callproc.c (call_process): Call prepare_to_modify_buffer in a single
        place, no longer delegating the task to insert_1_both, etc.  Call
        signal_after_change in each of two code branches, such that
        before-change-functions and after-change-functions are always called in
        balanced pairs.

Alan, is this behavior expected?

Thanks,
Clément.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39585; Package emacs. (Thu, 13 Feb 2020 19:16:02 GMT) Full text and rfc822 format available.

Message #8 received at 39585 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
Cc: 39585 <at> debbugs.gnu.org
Subject: Re: bug#39585: after-change-functions called with invalid positions
 in call-process
Date: Thu, 13 Feb 2020 19:15:19 +0000
Hello, Clément.

On Wed, Feb 12, 2020 at 22:56:49 -0500, Clément Pit-Claudel wrote:
> Hi all,

> Recent changes in call-process have introduced surprising behavior in
> after-change-functions (and caused bugs in flycheck, e.g.
> https://github.com/flycheck/flycheck/issues/1677).  From the
> documentation, I expected that functions added to the
> after-change-functions hook would be called only with valid positions.
> However, the following snippets shows that it's not the case:

>     (defun ~/after-change (beg end len)
>       (message "(after-change %S %S %S); (buffer %S %S %S)"
>                beg end len (point-min) (point-max) (buffer-size)))

>     (with-current-buffer (get-buffer-create "*tmp*")
>       (make-variable-buffer-local 'after-change-functions)
>       (add-hook 'after-change-functions #'~/after-change)
>       (call-process "echo" nil t t "Hello"))

> Running it repeatedly, this is what I observe:

>     (after-change 7 13 0); (buffer 1 7 6)
>     (after-change 13 19 0); (buffer 1 13 12)
>     (after-change 19 25 0); (buffer 1 19 18)
>     (after-change 25 31 0); (buffer 1 25 24)

> Note how each time the after-change-functions hook is called with a
> region past the end of the buffer.  It's as if after-change-functions
> was in fact call right before the insertion, instead of after.

Well, something like that.  Sorry about it, and thanks for drawing my
attention to it.

> Previous versions of Emacs didn't call after-change-functions in this
> case; it seems that the new behavior was introduced by this commit:

>     commit 224e8d146485ce178086549d41fa8359dcc0e03e
>     Author: Alan Mackenzie <acm <at> muc.de>
>     Date:   Wed Jan 22 19:50:30 2020 +0000

>         Make call_process call signal_after_change.  This fixes bug #38691.

>         Now, functions such as call-proess-region invoke after-change-functions
>         correctly.

>         * src/callproc.c (call_process): Call prepare_to_modify_buffer in a single
>         place, no longer delegating the task to insert_1_both, etc.  Call
>         signal_after_change in each of two code branches, such that
>         before-change-functions and after-change-functions are always called in
>         balanced pairs.

> Alan, is this behavior expected?

No, it is not.  I've applied this corrective patch to the emacs-27
branch:



commit d1e8ce8bb6fadf3d034ae437ff1c1b81be7d5209
Author: Alan Mackenzie <acm <at> muc.de>
Date:   Thu Feb 13 19:00:36 2020 +0000

    Make after-change-functions called from call-process get the correct BEG
    
    This fixes bug #39585.
    
    * src/callproc.c (call_process): Supply the correct CHARPOS to
    signal_after_change (twice).

diff --git a/src/callproc.c b/src/callproc.c
index 07dcc4c3ae..8883415f3f 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -811,7 +811,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
 		   && ! CODING_MAY_REQUIRE_DECODING (&process_coding))
             {
               insert_1_both (buf, nread, nread, 0, 0, 0);
-              signal_after_change (PT, 0, nread);
+              signal_after_change (PT - nread, 0, nread);
             }
 	  else
 	    {			/* We have to decode the input.  */
@@ -854,7 +854,8 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
 
 	      TEMP_SET_PT_BOTH (PT + process_coding.produced_char,
 				PT_BYTE + process_coding.produced);
-              signal_after_change (PT, 0, process_coding.produced_char);
+              signal_after_change (PT - process_coding.produced_char,
+                                   0, process_coding.produced_char);
 	      carryover = process_coding.carryover_bytes;
 	      if (carryover > 0)
 		memcpy (buf, process_coding.carryover,



.  It should find its way into the master branch in the usual way.
Please let me know whether you agree with me that this bug can now be
closed.  Thanks!

> Thanks,
> Clément.

-- 
Alan Mackenzie (Nuremberg, Germany).




bug Marked as fixed in versions 27.1. Request was from Noam Postavsky <npostavs <at> gmail.com> to control <at> debbugs.gnu.org. (Mon, 24 Feb 2020 18:24:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39585; Package emacs. (Sun, 20 Sep 2020 09:51:01 GMT) Full text and rfc822 format available.

Message #13 received at 39585 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: 39585 <at> debbugs.gnu.org
Subject: Re: control message for bug #39585
Date: Sun, 20 Sep 2020 11:49:54 +0200
Noam Postavsky <npostavs <at> gmail.com> writes:

> # should be fixed, pending verification from reporter
> fixed 39585 27.1

This was half a year ago, so I'm assuming the fix really worked (even if
there was no verification from the reporter), and I'm closing this bug
report.  If the problem persists, please respond to the debbugs address
and we'll reopen.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




bug closed, send any further explanations to 39585 <at> debbugs.gnu.org and Clément Pit-Claudel <cpitclaudel <at> gmail.com> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Sun, 20 Sep 2020 09:51:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 18 Oct 2020 11:24:11 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 163 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.