GNU bug report logs - #73018
31.0.50; wdired + replace-regexp only modifies the visible portion of the buffer

Previous Next

Package: emacs;

Reported by: Madhu <enometh <at> meer.net>

Date: Wed, 4 Sep 2024 02:38:01 UTC

Severity: normal

Found in version 31.0.50

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: michael_heerdegen <at> web.de, enometh <at> meer.net, 73018-done <at> debbugs.gnu.org, yantar92 <at> posteo.net, juri <at> linkov.net
Subject: bug#73018: 31.0.50; wdired + replace-regexp only modifies the visible portion of the buffer
Date: Sun, 22 Sep 2024 23:43:03 -0400
[Message part 1 (text/plain, inline)]
> bug#65451, sigh).  I did install the change you suggested on master.
> And with that, I'm closing this bug.

BTW, for `master` I think a better change is to fix the check.
E.g. with the patch below.


        Stefan
[search.patch (text/x-diff, inline)]
diff --git a/src/search.c b/src/search.c
index 24b1ee6bd3f..718d23b919e 100644
--- a/src/search.c
+++ b/src/search.c
@@ -2356,6 +2356,18 @@ DEFUN ("posix-search-forward", Fposix_search_forward, Sposix_search_forward, 1,
 {
   return search_command (regexp, bound, noerror, count, 1, true, true);
 }
+
+/* Return the number of search regs currently used.
+   FIXME: Maybe we should keep this as a field in `search_regs` rather
+   than (re)computing it dynamically.  */
+static ptrdiff_t
+search_regs_num_used (void)
+{
+  ptrdiff_t last = search_regs.num_regs - 1;
+  while (last >= 0 && search_regs.start[last] < 0)
+    last--;
+  return last + 1;
+}
 
 DEFUN ("replace-match", Freplace_match, Sreplace_match, 1, 5, 0,
        doc: /* Replace text matched by last search with NEWTEXT.
@@ -2759,11 +2771,22 @@ DEFUN ("replace-match", Freplace_match, Sreplace_match, 1, 5, 0,
     }
 
   newpoint = sub_start + SCHARS (newtext);
+  ptrdiff_t num_used = search_regs_num_used ();
 
   /* Replace the old text with the new in the cleanest possible way.  */
   replace_range (sub_start, sub_end, newtext, 1, 0, 1, true, true);
   signal_after_change (sub_start, sub_end - sub_start, SCHARS (newtext));
 
+  if (search_regs_num_used () != num_used)
+    /* The replace_range etc. functions can trigger modification hooks
+       (see signal_before_change and signal_after_change).  Try to error
+       out if these hooks clobber the match data since clobbering can
+       result in confusing bugs.  We used to check for changes in
+       search_regs start and end, but that fails if modification hooks
+       remove or add text earlier in the buffer, so just check num_regs
+       now. */
+    error ("Match data clobbered by buffer modification hooks");
+
   if (case_action == all_caps)
     Fupcase_region (make_fixnum (search_regs.start[sub]),
 		    make_fixnum (newpoint),
@@ -2869,7 +2892,8 @@ DEFUN ("match-data", Fmatch_data, Smatch_data, 0, 3, 0,
 {
   Lisp_Object tail, prev;
   Lisp_Object *data;
-  ptrdiff_t i, len;
+  ptrdiff_t i, used = search_regs_num_used () + 1;
+  ptrdiff_t len = used * 2;
 
   if (!NILP (reseat))
     for (tail = reuse; CONSP (tail); tail = XCDR (tail))
@@ -2885,10 +2909,9 @@ DEFUN ("match-data", Fmatch_data, Smatch_data, 0, 3, 0,
   prev = Qnil;
 
   USE_SAFE_ALLOCA;
-  SAFE_NALLOCA (data, 1, 2 * search_regs.num_regs + 1);
+  SAFE_NALLOCA (data, 1, 2 * used + 1);
 
-  len = 0;
-  for (i = 0; i < search_regs.num_regs; i++)
+  for (i = 0; i < used; i++)
     {
       ptrdiff_t start = search_regs.start[i];
       if (start >= 0)
@@ -2913,8 +2936,6 @@ DEFUN ("match-data", Fmatch_data, Smatch_data, 0, 3, 0,
 	  else
 	    /* last_thing_searched must always be Qt, a buffer, or Qnil.  */
 	    emacs_abort ();
-
-	  len = 2 * i + 2;
 	}
       else
 	data[2 * i] = data[2 * i + 1] = Qnil;

This bug report was last modified 295 days ago.

Previous Next


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