GNU bug report logs - #78042
31.0.50; Improper sequence of `before/after-change-functions` calls

Previous Next

Full log

View this message in rfc822 format

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 78042 <at> debbugs.gnu.org
Subject: bug#78042: Improper sequence of `before/after-change-functions` calls
Date: Thu, 24 Apr 2025 21:35:09 -0400

[Message part 1 (text/plain, inline)]

Hmm...

The code that runs those hooks is really littered at odd places, with no
good/standard way to control when to run it and when not.
I suggest the patch below (which includes a corresponding test).
Any objection?


        Stefan

[decode-change-hook.patch (text/x-diff, inline)]

diff --git a/src/coding.c b/src/coding.c
index 63b0dbeb18b..b4209986918 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -7363,6 +7363,7 @@ decode_coding (struct coding_system *coding)
   struct ccl_spec cclspec;
   int carryover;
   int i;
+  specpdl_ref count = SPECPDL_INDEX ();
 
   USE_SAFE_ALLOCA;
 
@@ -7389,6 +7390,8 @@ decode_coding (struct coding_system *coding)
 
       undo_list = BVAR (current_buffer, undo_list);
       bset_undo_list (current_buffer, Qt);
+      /* The caller should run *-change-functions over the whole region.  */
+      specbind (Qinhibit_modification_hooks, Qt);
     }
 
   coding->consumed = coding->consumed_char = 0;
@@ -7501,7 +7504,7 @@ decode_coding (struct coding_system *coding)
       record_insert (coding->dst_pos, coding->produced_char);
     }
 
-  SAFE_FREE ();
+  SAFE_FREE_UNBIND_TO (count, Qnil);
 }
 
 
diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el
index 2553ad3ec2c..9a27c420f1e 100644
--- a/test/src/editfns-tests.el
+++ b/test/src/editfns-tests.el
@@ -498,10 +498,10 @@ sanity-check-change-functions-verbose
 (defvar sanity-check-change-functions-op nil)
 (defmacro sanity-check-change-functions-with-op (op &rest body)
   (declare (debug t) (indent 1))
-  `(let ((sanity-check-change-functions-op ,op))
-     (sanity-check--message "%S..." sanity-check-change-functions-op)
+  `(let ((sanity-check-change-functions-op (list ,op)))
+     (sanity-check--message "%S..." ,op)
      ,@body
-     (sanity-check--message "%S...done" sanity-check-change-functions-op)))
+     (sanity-check--message "%S...done" ,op)))
 
 (defun sanity-check--message (&rest args)
   (if sanity-check-change-functions-verbose (apply #'message args)))
@@ -530,6 +530,7 @@ sanity-check-change-functions-check-size
     (setq sanity-check-change-functions-buffer-size (buffer-size)))))
 
 (defun sanity-check-change-functions-before (beg end)
+  (push `(BEFORE ,beg ,end) sanity-check-change-functions-op)
   (sanity-check--message "Before: %S %S" beg end)
   (unless (<= (point-min) beg end (point-max))
     (sanity-check-change-functions-error
@@ -540,6 +541,7 @@ sanity-check-change-functions-before
   (setq sanity-check-change-functions-end end))
 
 (defun sanity-check-change-functions-after (beg end len)
+  (push `(AFTER ,beg ,end ,len) sanity-check-change-functions-op)
   (sanity-check--message "After : %S %S (%S)" beg end len)
   (unless (<= (point-min) beg end (point-max))
     (sanity-check-change-functions-error
@@ -565,7 +567,7 @@ sanity-check-change-functions-after
 (defun sanity-check-change-functions-errors ()
   (sanity-check-change-functions-check-size)
   (if sanity-check-change-functions-errors
-      (cons sanity-check-change-functions-op
+      (cons (reverse sanity-check-change-functions-op)
             sanity-check-change-functions-errors)))
 
 (ert-deftest editfns-tests--before/after-change-functions ()
@@ -591,6 +593,24 @@ editfns-tests--before/after-change-functions
         (decode-coding-region beg (point) 'utf-8)
         (should (null (sanity-check-change-functions-errors)))))
 
+    (let ((beg (point)))                ;bug#78042
+      (apply #'insert (make-list 5000 "hell\351 "))
+      (sanity-check-change-functions-with-op 'DECODE-CODING-LARGE-REGION
+        (decode-coding-region beg (point) 'windows-1252)
+        (should-not (sanity-check-change-functions-errors))))
+
+    (let ((beg (point)))                ;bug#78042
+      (sanity-check-change-functions-with-op 'DECODE-CODING-INSERT
+        ;; The `insert' calls make sure we track the buffer-size
+        ;; so as to detect if `decode-coding-string' fails to run the
+        ;; `*-change-functions'.
+        (insert "<")
+        (decode-coding-string "hell\351 " 'windows-1252 nil (current-buffer))
+        (forward-char 6)
+        (insert ">")
+        (should (equal "<hellé >" (buffer-substring beg (point))))
+        (should-not (sanity-check-change-functions-errors))))
+
     (sanity-check-change-functions-with-op 'ENCODE-CODING-STRING
       (encode-coding-string "ééé" 'utf-8 nil (current-buffer))
       (should (null (sanity-check-change-functions-errors))))

Previous Next

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