From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.bugs Subject: bug#29349: [Patch] Bug 29349: read_key_sequence is only partially recursive. This is a bug. Date: Sun, 19 Nov 2017 15:59:08 +0000 Message-ID: <20171119155908.GB4576@ACM> References: <20171118093843.GA3819@ACM> <20171119123456.GA4576@ACM> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1511107404 12169 195.159.176.226 (19 Nov 2017 16:03:24 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 19 Nov 2017 16:03:24 +0000 (UTC) User-Agent: Mutt/1.7.2 (2016-11-26) To: 29349@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Nov 19 17:03:18 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eGS3r-0002U4-AL for geb-bug-gnu-emacs@m.gmane.org; Sun, 19 Nov 2017 17:03:11 +0100 Original-Received: from localhost ([::1]:53490 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eGS3y-0004Q5-Jw for geb-bug-gnu-emacs@m.gmane.org; Sun, 19 Nov 2017 11:03:18 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:39642) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eGS3n-0004Mx-1x for bug-gnu-emacs@gnu.org; Sun, 19 Nov 2017 11:03:08 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eGS3i-0001YG-U0 for bug-gnu-emacs@gnu.org; Sun, 19 Nov 2017 11:03:07 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:39013) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eGS3i-0001YB-Pk for bug-gnu-emacs@gnu.org; Sun, 19 Nov 2017 11:03:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1eGS3i-0003d2-BB for bug-gnu-emacs@gnu.org; Sun, 19 Nov 2017 11:03:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Alan Mackenzie Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 19 Nov 2017 16:03:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 29349 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 29349-submit@debbugs.gnu.org id=B29349.151110733013891 (code B ref 29349); Sun, 19 Nov 2017 16:03:02 +0000 Original-Received: (at 29349) by debbugs.gnu.org; 19 Nov 2017 16:02:10 +0000 Original-Received: from localhost ([127.0.0.1]:47694 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eGS2r-0003by-Q8 for submit@debbugs.gnu.org; Sun, 19 Nov 2017 11:02:10 -0500 Original-Received: from ocolin.muc.de ([193.149.48.4]:37714 helo=mail.muc.de) by debbugs.gnu.org with smtp (Exim 4.84_2) (envelope-from ) id 1eGS2p-0003br-TJ for 29349@debbugs.gnu.org; Sun, 19 Nov 2017 11:02:08 -0500 Original-Received: (qmail 62760 invoked by uid 3782); 19 Nov 2017 16:02:05 -0000 Original-Received: from acm.muc.de (p548C7514.dip0.t-ipconnect.de [84.140.117.20]) by colin.muc.de (tmda-ofmipd) with ESMTP; Sun, 19 Nov 2017 17:02:05 +0100 Original-Received: (qmail 9577 invoked by uid 1000); 19 Nov 2017 15:59:08 -0000 Content-Disposition: inline In-Reply-To: <20171119123456.GA4576@ACM> X-Delivery-Agent: TMDA/1.1.12 (Macallan) X-Primary-Address: acm@muc.de X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:140096 Archived-At: Hello, Emacs. On Sun, Nov 19, 2017 at 12:34:56 +0000, Alan Mackenzie wrote: > On Sat, Nov 18, 2017 at 09:38:43 +0000, Alan Mackenzie wrote: > > In branch emacs-26. > > I came across this bug whilst working on bug #29272 ("C-h k C-mouse-3" > > followed by menu selection asks for more keys). > > In a Linux tty using the GPM mouse package, doing read_key_sequence (the > > C function in keyboard.c), when a menu action is activated, > > read_key_sequence calls itself recursively to handle all the mouse > > manipulation. > > Unfortunately, the variable raw_keybuf_count is initialised to 0 in > > r_k_s. This includes in the recursive call. This variable indexes the > > global buffer raw_keybuf, which accumulates the raw events which make up > > the key sequence. > > The result of this is that the events in the recursive call overwrite > > the stored events of the outer r_k_s call, leaving a mess. > > r_k_s is static in keyboard.c and is called from three functions: > > command_loop_1, read_menu_command (the one that gives the trouble), and > > read_key_sequence_vs. > > So I propose as a solution that raw_keybuf_count be initialised to zero > > in two of these three functions, but not in read_menu_command (and no > > longer in read_key_sequence). I've tried this, and it seems to work. > > It has the disadvantage of being ugly, and it makes read_menu_command > > only callable as a subfunction of r_k_s. > > Has anybody any thoughts on this? > Here is how I propose to solve this: > (i) In keyboard.c, the static variables raw_keybuf and raw_keybuf_count > will become pointers. They will initially point to a static buffer and > a static integer. For safety's sake, they will be reinitialised to > those static variables in command_loop_1(), just before the invocation > of read_key_sequence(). > (ii) read_key_sequence() will get a Lisp_Object buffer and a count > variable as local variables. Around the call to read_char(), > raw_keybuf{,_count} will be set to point to these locals, so that should > read_char encounter a menu, its events will be stored in the local copy > of the Lisp_Object buffer. > (iii) On return from read_char, if any events are in the local buffer, > they will be appended to the buffer in the enclosing scope. The global > pointers raw_keybuf{,_count} will be restored to their previous values. > In short, raw_keybuf and raw_keybuf_count will be "bound" to local > variables around the call to read_char(). > Comments? Here's a patch which implements the above: diff --git a/src/keyboard.c b/src/keyboard.c index 57757cf211..ae687aad89 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -121,12 +121,17 @@ ptrdiff_t this_command_key_count; /* This vector is used as a buffer to record the events that were actually read by read_key_sequence. */ -static Lisp_Object raw_keybuf; -static int raw_keybuf_count; - -#define GROW_RAW_KEYBUF \ - if (raw_keybuf_count == ASIZE (raw_keybuf)) \ - raw_keybuf = larger_vector (raw_keybuf, 1, -1) +static Lisp_Object raw_keybuf_buffer; +static int raw_keybuf_count_buffer; +static Lisp_Object *raw_keybuf = &raw_keybuf_buffer; +static int *raw_keybuf_count = &raw_keybuf_count_buffer; + +#define GROW_RAW_KEYBUF(inc) \ + if (*raw_keybuf_count > ASIZE (*raw_keybuf) - (inc)) \ + *raw_keybuf = \ + larger_vector (*raw_keybuf, \ + (inc) + *raw_keybuf_count - ASIZE (*raw_keybuf), \ + -1) /* Number of elements of this_command_keys that precede this key sequence. */ @@ -1365,6 +1370,8 @@ command_loop_1 (void) Vthis_command_keys_shift_translated = Qnil; /* Read next key sequence; i gets its length. */ + raw_keybuf_count = &raw_keybuf_count_buffer; /* For safety */ + raw_keybuf = &raw_keybuf_buffer; /* Ditto */ i = read_key_sequence (keybuf, ARRAYELTS (keybuf), Qnil, 0, 1, 1, 0); @@ -8910,6 +8917,11 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt, /* How many keys there are in the current key sequence. */ int t; + int *outer_raw_keybuf_count; + Lisp_Object *outer_raw_keybuf; + int inner_raw_keybuf_count_buffer; + Lisp_Object inner_raw_keybuf_buffer = Fmake_vector (make_number (30), Qnil); + /* The length of the echo buffer when we started reading, and the length of this_command_keys when we started reading. */ ptrdiff_t echo_start UNINIT; @@ -8971,7 +8983,7 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt, /* List of events for which a fake prefix key has been generated. */ Lisp_Object fake_prefixed_keys = Qnil; - raw_keybuf_count = 0; + *raw_keybuf_count = 0; last_nonmenu_event = Qnil; @@ -9142,11 +9154,23 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt, { KBOARD *interrupted_kboard = current_kboard; struct frame *interrupted_frame = SELECTED_FRAME (); + int i; + outer_raw_keybuf_count = raw_keybuf_count; + outer_raw_keybuf = raw_keybuf; + inner_raw_keybuf_count_buffer = 0; + raw_keybuf_count = &inner_raw_keybuf_count_buffer; + raw_keybuf = &inner_raw_keybuf_buffer; /* Calling read_char with COMMANDFLAG = -2 avoids redisplay in read_char and its subroutines. */ key = read_char (prevent_redisplay ? -2 : NILP (prompt), current_binding, last_nonmenu_event, &used_mouse_menu, NULL); + raw_keybuf_count = outer_raw_keybuf_count; + raw_keybuf = outer_raw_keybuf; + GROW_RAW_KEYBUF (inner_raw_keybuf_count_buffer); + for (i = 0; i < inner_raw_keybuf_count_buffer; i++) + ASET (*raw_keybuf, (*raw_keybuf_count)++, + AREF (inner_raw_keybuf_buffer, i)); if ((INTEGERP (key) && XINT (key) == -2) /* wrong_kboard_jmpbuf */ /* When switching to a new tty (with a new keyboard), read_char returns the new buffer, rather than -2 @@ -9254,9 +9278,9 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt, && XINT (key) == quit_char && current_buffer != starting_buffer) { - GROW_RAW_KEYBUF; - ASET (raw_keybuf, raw_keybuf_count, key); - raw_keybuf_count++; + GROW_RAW_KEYBUF (1); + ASET (*raw_keybuf, *raw_keybuf_count, key); + (*raw_keybuf_count)++; keybuf[t++] = key; mock_input = t; Vquit_flag = Qnil; @@ -9295,9 +9319,9 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt, current_binding = active_maps (first_event); } - GROW_RAW_KEYBUF; - ASET (raw_keybuf, raw_keybuf_count, key); - raw_keybuf_count++; + GROW_RAW_KEYBUF (1); + ASET (*raw_keybuf, *raw_keybuf_count, key); + (*raw_keybuf_count)++; } /* Clicks in non-text areas get prefixed by the symbol @@ -9343,8 +9367,9 @@ read_key_sequence (Lisp_Object *keybuf, int bufsize, Lisp_Object prompt, && BUFFERP (XWINDOW (window)->contents) && XBUFFER (XWINDOW (window)->contents) != current_buffer) { - ASET (raw_keybuf, raw_keybuf_count, key); - raw_keybuf_count++; + GROW_RAW_KEYBUF (1); + ASET (*raw_keybuf, *raw_keybuf_count, key); + (*raw_keybuf_count)++; keybuf[t] = key; mock_input = t + 1; @@ -10117,7 +10142,7 @@ shows the events before all translations (except for input methods). The value is always a vector. */) (void) { - return Fvector (raw_keybuf_count, XVECTOR (raw_keybuf)->contents); + return Fvector (*raw_keybuf_count, XVECTOR (*raw_keybuf)->contents); } DEFUN ("clear-this-command-keys", Fclear_this_command_keys, @@ -11262,8 +11287,8 @@ syms_of_keyboard (void) this_command_keys = Fmake_vector (make_number (40), Qnil); staticpro (&this_command_keys); - raw_keybuf = Fmake_vector (make_number (30), Qnil); - staticpro (&raw_keybuf); + raw_keybuf_buffer = Fmake_vector (make_number (30), Qnil); + staticpro (raw_keybuf); DEFSYM (Qcommand_execute, "command-execute"); DEFSYM (Qinternal_echo_keystrokes_prefix, "internal-echo-keystrokes-prefix"); -- Alan Mackenzie (Nuremberg, Germany).