From 4f27eb41c638270a2c421126382cbb5073986288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Braun?= Date: Sun, 31 Mar 2024 12:54:33 +0200 Subject: [PATCH] transpose-regions: fix crash, add tests for text properties. The tests contain non-ASCII characters and text properties, and crash previous Emacs versions. No longer confuse lengths in bytes and characters: 1. For consistent naming split len_mid into len_mid and len_mid_byte. 2. Remove the case len1_byte == len2_byte: only one memmove() could be optimized out compared to the other cases. Even tmp_interval_mid is necessary, as the intervals need position adjustments. Don't bother with different undo records in this case. --- src/editfns.c | 52 +++++++++----------------------------- test/src/editfns-tests.el | 53 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 40 deletions(-) diff --git a/src/editfns.c b/src/editfns.c index 85f7739df07..ba589ee497e 100644 --- a/src/editfns.c +++ b/src/editfns.c @@ -4495,6 +4495,7 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5, { register ptrdiff_t start1, end1, start2, end2; ptrdiff_t start1_byte, start2_byte, len1_byte, len2_byte, end2_byte; + ptrdiff_t len_mid_byte; ptrdiff_t gap, len1, len_mid, len2; unsigned char *start1_addr, *start2_addr, *temp; @@ -4653,42 +4654,10 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5, /* Non-adjacent regions, because end1 != start2, bleagh... */ else { - len_mid = start2_byte - (start1_byte + len1_byte); + len_mid_byte = start2_byte - (start1_byte + len1_byte); + len_mid = start2 - end1; - if (len1_byte == len2_byte) - /* Regions are same size, though, how nice. */ - { - USE_SAFE_ALLOCA; - - modify_text (start1, end2); - record_change (start1, len1); - record_change (start2, len2); - tmp_interval1 = copy_intervals (cur_intv, start1, len1); - tmp_interval2 = copy_intervals (cur_intv, start2, len2); - - tmp_interval3 = validate_interval_range (buf, &startr1, &endr1, 0); - if (tmp_interval3) - set_text_properties_1 (startr1, endr1, Qnil, buf, tmp_interval3); - - tmp_interval3 = validate_interval_range (buf, &startr2, &endr2, 0); - if (tmp_interval3) - set_text_properties_1 (startr2, endr2, Qnil, buf, tmp_interval3); - - temp = SAFE_ALLOCA (len1_byte); - start1_addr = BYTE_POS_ADDR (start1_byte); - start2_addr = BYTE_POS_ADDR (start2_byte); - memcpy (temp, start1_addr, len1_byte); - memcpy (start1_addr, start2_addr, len2_byte); - memcpy (start2_addr, temp, len1_byte); - SAFE_FREE (); - - graft_intervals_into_buffer (tmp_interval1, start2, - len1, current_buffer, 0); - graft_intervals_into_buffer (tmp_interval2, start1, - len2, current_buffer, 0); - } - - else if (len1_byte < len2_byte) /* Second region larger than first */ + if (len1_byte < len2_byte) /* Second region larger than first */ /* Non-adjacent & unequal size, area between must also be shifted. */ { USE_SAFE_ALLOCA; @@ -4708,8 +4677,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5, start1_addr = BYTE_POS_ADDR (start1_byte); start2_addr = BYTE_POS_ADDR (start2_byte); memcpy (temp, start2_addr, len2_byte); - memcpy (start1_addr + len_mid + len2_byte, start1_addr, len1_byte); - memmove (start1_addr + len2_byte, start1_addr + len1_byte, len_mid); + memcpy (start1_addr + len_mid_byte + len2_byte, start1_addr, len1_byte); + memmove (start1_addr + len2_byte, start1_addr + len1_byte, len_mid_byte); memcpy (start1_addr, temp, len2_byte); SAFE_FREE (); @@ -4721,7 +4690,10 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5, len2, current_buffer, 0); } else - /* Second region smaller than first. */ + /* Second region not larger than first. */ + /* The equal-length case is not simpler, + the position stored in intervals for the text inbetween + still needs adjustment. */ { USE_SAFE_ALLOCA; @@ -4742,8 +4714,8 @@ DEFUN ("transpose-regions", Ftranspose_regions, Stranspose_regions, 4, 5, start2_addr = BYTE_POS_ADDR (start2_byte); memcpy (temp, start1_addr, len1_byte); memcpy (start1_addr, start2_addr, len2_byte); - memmove (start1_addr + len2_byte, start1_addr + len1_byte, len_mid); - memcpy (start1_addr + len2_byte + len_mid, temp, len1_byte); + memmove (start1_addr + len2_byte, start1_addr + len1_byte, len_mid_byte); + memcpy (start1_addr + len2_byte + len_mid_byte, temp, len1_byte); SAFE_FREE (); graft_intervals_into_buffer (tmp_interval1, end2 - len1, diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el index b3b7da65ad3..82f0bcd1e71 100644 --- a/test/src/editfns-tests.el +++ b/test/src/editfns-tests.el @@ -132,6 +132,59 @@ propertize/error-even-number-of-args "Number of args for `propertize' must be odd." (should-error (propertize "foo" 'bar) :type 'wrong-number-of-arguments)) +;; Tests for `transpose-region' + +(ert-deftest transpose-regions-text-properties2 () + "Test `transpose-regions' thoroughly with text properties." + (let* ((string1pre (propertize "a\x2013" :test 1)) + (middle (propertize "c\x00e9h" :test 0)) + (string2 (propertize "\x25cf\x25cb" :test 2)) + (bytes1 (string-bytes string1pre)) + (bytes2 (string-bytes string2))) + ;; (cl-assert (< bytes1 bytes2)) + (dotimes (i (+ 3 (- bytes2 bytes1))) + (let ((string1 (concat string1pre + (propertize (make-string i ?X) + :test t)))) + (with-temp-buffer + (insert string1 middle string2) + (buffer-enable-undo) + (transpose-regions + 1 (1+ (length string1)) + (- (point) (length string2)) (point)) + (should (equal-including-properties + (buffer-string) + (concat string2 middle string1))) + (undo-boundary) + (let ((this-command #'undo) + (last-command #'ert)) ; anything but undo + (undo)) + (should (equal-including-properties + (buffer-string) + (concat string1 middle string2)))))))) + +(ert-deftest transpose-regions-text-properties () + "Test `transpose-regions' with text properties. +This test is known to crash Emacs 28.2, 29.2, 29.3." + (with-temp-buffer + (insert (propertize "a" 'face 'font-lock-variable-name-face)) + (insert ":\n") + (insert (propertize "b" 'face 'font-lock-variable-name-face)) + (insert ": \x2113\x2080\n") + (insert (propertize "v" 'face 'font-lock-variable-name-face)) + (insert ": scaling\n") + ;; Move last line to the beginning + (transpose-regions 1 1 10 21) + (should (equal-including-properties + (buffer-string) + (concat + (propertize "v" 'face 'font-lock-variable-name-face) + ": scaling\n" + (propertize "a" 'face 'font-lock-variable-name-face) + ":\n" + (propertize "b" 'face 'font-lock-variable-name-face) + ": \x2113\x2080\n"))))) + ;; Tests for bug#5131. (defun transpose-test-reverse-word (start end) "Reverse characters in a word by transposing pairs of characters." -- 2.39.2