Subject: Re: bug#70122: 29.3.50; transpose-regions can crash Emacs

From: Eli Zaretskii <[email-masked]>
Date: Mon, 01 Apr 2024 16:17:04 +0300

> Cc: [email-masked]
> Date: Mon, 01 Apr 2024 14:55:21 +0300
> From: Eli Zaretskii <[email-masked]>
> 
> > Please find attached a patch to transpose-regions
> > with added tests.  (The patch intends to fix typos:
> > makes sure lengths in bytes and characters are not confused.
> 
> Thanks, but could you please show the minimal change required to fix
> just the particular problem with this scenario (and perhaps explain
> the reason for the crash in words), without any cleanup and
> typo/confusion fixes?  That would make it easier to review the patch,
> whereas with what you sent, it is hard to understand what exactly is
> being fixed.

I came up with the patch below.  The problem is that len_mid, which is
a difference between byte positions, was sometimes used in calls that
expect differences between character positions instead.  The patch
below passes both your test and the already-existing tests in
test/src/editfns-tests.el.  WDYT?

> > One case (likely for optimization only) has been removed,
> > seemed too much trouble to get it right.)
> 
> If you explain the reason for the crash, perhaps we could leave the
> optimization alone.

I found that len_mid was not used in that branch, so we can leave it
alone.

Here's the patch:

diff --git a/src/editfns.c b/src/editfns.c
index 4ccf765..124fd47 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4677,7 +4677,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           modify_text (start1, end2);
           record_change (start1, (end2 - start1));
           tmp_interval1 = copy_intervals (cur_intv, start1, len1);
-          tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
+          tmp_interval_mid = copy_intervals (cur_intv, end1,
+					     start2 - (start1 + len1));
           tmp_interval2 = copy_intervals (cur_intv, start2, len2);
 
 	  tmp_interval3 = validate_interval_range (buf, &startr1, &endr2, 0);
@@ -4697,7 +4698,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           graft_intervals_into_buffer (tmp_interval1, end2 - len1,
                                        len1, current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval_mid, start1 + len2,
-                                       len_mid, current_buffer, 0);
+                                       start2 - (start1 + len1),
+				       current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval2, start1,
                                        len2, current_buffer, 0);
         }
@@ -4710,7 +4712,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           modify_text (start1, end2);
 
           tmp_interval1 = copy_intervals (cur_intv, start1, len1);
-          tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
+          tmp_interval_mid = copy_intervals (cur_intv, end1,
+					     start2 - (start1 + len1));
           tmp_interval2 = copy_intervals (cur_intv, start2, len2);
 
 	  tmp_interval3 = validate_interval_range (buf, &startr1, &endr2, 0);
@@ -4730,7 +4733,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5,
           graft_intervals_into_buffer (tmp_interval1, end2 - len1,
                                        len1, current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval_mid, start1 + len2,
-                                       len_mid, current_buffer, 0);
+                                       start2 - (start1 + len1),
+				       current_buffer, 0);
           graft_intervals_into_buffer (tmp_interval2, start1,
                                        len2, current_buffer, 0);
         }