GNU bug report logs - #28039
26.0.50; Ses copy-region-as-kill advice assumes non-nil beg end arguments

Previous Next

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

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

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

From: Vitalie Spinu <spinuvit <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.50;
 Ses copy-region-as-kill advice assumes non-nil beg end arguments
Date: Thu, 10 Aug 2017 18:42:37 +0200

Hi,

Once ses is loaded I am getting the following error on kill in magit buffers:

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
  ses--advice-copy-region-as-kill(#[770 "\
  apply(ses--advice-copy-region-as-kill #[770 
  copy-region-as-kill(nil nil region)
  magit-copy-buffer-revision()
  funcall-interactively(magit-copy-buffer-revision)
  #<subr call-interactively>(magit-copy-buffer-revision nil nil)
  apply(#<subr call-interactively> magit-copy-buffer-revision (nil nil))
  call-interactively <at> ido-cr+-record-current-command(#<subr call-interactively> magit-copy-buffer-revision nil nil)
  apply(call-interactively <at> ido-cr+-record-current-command #<subr call-interactively> (magit-copy-buffer-revision nil nil))
  call-interactively(magit-copy-buffer-revision nil nil)
  command-execute(magit-copy-buffer-revision)


Magit calls (copy-region-as-kill nil nil 'region). According to the doc first
two args should be ignored when 'region is passed. So I would assume it's the
bug in `ses--advice-copy-region-as-kill` then.


Thanks,

  Vitalie



In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.18.9)
 of 2017-06-28 built on galago
Repository revision: 5d45ba1a05bccc53d52422e867f378a0adeb8970
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description:	Ubuntu 16.04.2 LTS

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Vitalie Spinu <spinuvit <at> gmail.com>
Cc: 28039 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: bug#28039: 26.0.50;
 Ses copy-region-as-kill advice assumes non-nil beg end arguments
Date: Sat, 12 Aug 2017 13:25:33 +0900

Vitalie Spinu <spinuvit <at> gmail.com> writes:


> Once ses is loaded I am getting the following error on kill in magit buffers:
>
> Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
>   ses--advice-copy-region-as-kill(#[770 "\
>   apply(ses--advice-copy-region-as-kill #[770 
>   copy-region-as-kill(nil nil region)
>   magit-copy-buffer-revision()
>   funcall-interactively(magit-copy-buffer-revision)
>   #<subr call-interactively>(magit-copy-buffer-revision nil nil)
>   apply(#<subr call-interactively> magit-copy-buffer-revision (nil nil))
>   call-interactively <at> ido-cr+-record-current-command(#<subr call-interactively> magit-copy-buffer-revision nil nil)
>   apply(call-interactively <at> ido-cr+-record-current-command #<subr call-interactively> (magit-copy-buffer-revision nil nil))
>   call-interactively(magit-copy-buffer-revision nil nil)
>   command-execute(magit-copy-buffer-revision)
>
>
> Magit calls (copy-region-as-kill nil nil 'region). According to the doc first
> two args should be ignored when 'region is passed. So I would assume it's the
> bug in `ses--advice-copy-region-as-kill` then.
Thank you for the report.
I've noticed that `copy-region-as-kill' also needs `natnump' checks for
BEG, END.
How about th following patch?

--8<-----------------------------cut here---------------start------------->8---
commit 019b5ebf09e69593b0e50ba2ce53501b83331ecc
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Sat Aug 12 13:24:35 2017 +0900

    copy-region-as-kill: Check types of BEG, END
    
    * lisp/ses.el (ses--advice-copy-region-as-kill): Check that BEG, END
    are non-negative integers (Bug#28039).
    (copy-region-as-kill): Same.
    * test/lisp/simple-tests.el (simple-test-bug28039): Add test.

diff --git a/lisp/ses.el b/lisp/ses.el
index 8c5ff2136f..676c36a5b7 100644
--- a/lisp/ses.el
+++ b/lisp/ses.el
@@ -3052,14 +3052,12 @@ ses--advice-copy-region-as-kill
 
 This advice also includes some SES-specific code because otherwise it's too
 hard to override how mouse-1 works."
-  (when (> beg end)
-    (let ((temp beg))
-      (setq beg end
-	    end temp)))
-  (if (not (and (derived-mode-p 'ses-mode)
-		(eq (get-text-property beg 'read-only) 'ses)
-		(eq (get-text-property (1- end) 'read-only) 'ses)))
+  (if (or (not (or (natnump beg) (natnump end)))
+          (not (and (derived-mode-p 'ses-mode)
+		    (eq (get-text-property beg 'read-only) 'ses)
+		    (eq (get-text-property (1- end) 'read-only) 'ses))))
       (apply crak-fun beg end args) ; Normal copy-region-as-kill.
+    (when (> beg end) (cl-rotatef beg end))
     (kill-new (ses-copy-region beg end))
     (if transient-mark-mode
 	(setq deactivate-mark t))
diff --git a/lisp/simple.el b/lisp/simple.el
index 933ffc55a6..170e612918 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -4552,7 +4552,7 @@ copy-region-as-kill
                  (funcall region-extract-function nil)
                (filter-buffer-substring beg end))))
   (if (eq last-command 'kill-region)
-        (kill-append str (< end beg))
+        (kill-append str (and (natnump beg) (natnump end) (< end beg)))
       (kill-new str)))
   (setq deactivate-mark t)
   nil)
diff --git a/test/lisp/simple-tests.el b/test/lisp/simple-tests.el
index ad7aee1db1..27e607ec13 100644
--- a/test/lisp/simple-tests.el
+++ b/test/lisp/simple-tests.el
@@ -497,5 +497,16 @@ simple-test-undo-with-switched-buffer
       (should (equal (line-number-at-pos 5) 3))
       (should (equal (line-number-at-pos 7) 4)))))
 
+(ert-deftest simple-test-bug28039 ()
+  "Test for http://debbugs.gnu.org/28039 ."
+  (with-temp-buffer
+    (insert "123456789abc")
+    (kill-region 1 10)
+    (set-mark (point))
+    (goto-char 1)
+    (copy-region-as-kill nil nil t)
+    (should (equal "abc" (current-kill 0)))))
+
+
 (provide 'simple-test)
 ;;; simple-test.el ends here
--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-08-12 built on calancha-pc
Repository revision: e3ed43f4ac667d39fffcc48cfbe97b074f9aa5c7

Message #15 received at 28039-done <at> debbugs.gnu.org (full text, mbox):

From: Óscar Fuentes <ofv <at> wanadoo.es>
To: 28039-done <at> debbugs.gnu.org
Subject: Re: 26.0.50;
 Ses copy-region-as-kill advice assumes non-nil beg end arguments
Date: Sun, 17 Dec 2017 05:52:52 +0100

Fixed in emacs-26 2e9eba201 and master 506270f9c8.

Previous Next

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