From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] master 5b5f441: read_key_sequence: correct the handling of raw_keybuf in recursive calls Date: Wed, 22 Nov 2017 19:25:22 +0000 Message-ID: <20171122192522.GA4288@ACM> References: <20171120181209.23553.97060@vcs0.savannah.gnu.org> <20171120181210.7946F20416@vcs0.savannah.gnu.org> <20171120195918.GB3917@ACM> <20171120203004.GC3917@ACM> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1511378986 17317 195.159.176.226 (22 Nov 2017 19:29:46 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 22 Nov 2017 19:29:46 +0000 (UTC) User-Agent: Mutt/1.7.2 (2016-11-26) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Nov 22 20:29:39 2017 Return-path: Envelope-to: ged-emacs-devel@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 1eHaiF-0003nR-IP for ged-emacs-devel@m.gmane.org; Wed, 22 Nov 2017 20:29:35 +0100 Original-Received: from localhost ([::1]:41019 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eHaiM-00058X-UO for ged-emacs-devel@m.gmane.org; Wed, 22 Nov 2017 14:29:42 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:39700) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eHaho-00058S-83 for emacs-devel@gnu.org; Wed, 22 Nov 2017 14:29:09 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eHahk-0006AH-Qw for emacs-devel@gnu.org; Wed, 22 Nov 2017 14:29:08 -0500 Original-Received: from colin.muc.de ([193.149.48.1]:16073 helo=mail.muc.de) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1eHahk-00069g-Es for emacs-devel@gnu.org; Wed, 22 Nov 2017 14:29:04 -0500 Original-Received: (qmail 17901 invoked by uid 3782); 22 Nov 2017 19:29:01 -0000 Original-Received: from acm.muc.de (p548C717C.dip0.t-ipconnect.de [84.140.113.124]) by colin.muc.de (tmda-ofmipd) with ESMTP; Wed, 22 Nov 2017 20:29:00 +0100 Original-Received: (qmail 5384 invoked by uid 1000); 22 Nov 2017 19:25:22 -0000 Content-Disposition: inline In-Reply-To: X-Delivery-Agent: TMDA/1.1.12 (Macallan) X-Primary-Address: acm@muc.de X-detected-operating-system: by eggs.gnu.org: FreeBSD 9.x [fuzzy] X-Received-From: 193.149.48.1 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:220367 Archived-At: Hello, Stefan. On Mon, Nov 20, 2017 at 16:43:52 -0500, Stefan Monnier wrote: > >> But this `read_char` is within read_key_sequence: this read_key_sequence > >> may have been called from anywhere, .... > > read_key_sequence is static in keyboard.c, and is called from precisely > > three places: command_loop_1, read_key_sequence_vs, and > > read_menu_command. > Right: the middle one corresponds to `read-key-sequence` which can be > called from "anywhere" (i.e. Elisp). That is surely not the problem. The problem would be if something aborted a read_key_sequence, and somehow started another one without going via command_loop_1. In the current formulation read_key_sequence_vs could have set the pertinent pointer variables to the static ones as part of its initialisation. I didn't do that because it seemed unneeded. But it would be harmless, since there surely cannot be a recursive call to read_key_sequence_vs. > >> .... so after we exit it (non-locally), we may end up running > >> arbitrary Elisp code before we return to command_loop_1, can't we? > > Is it possible to exit non-locally from read_char (or one of its called > > subroutines)? > `read_char` will run timers and process filters. See for example > `read-key` which calls `read-key-sequence` with a timer that makes it > exit by throwing `read-key` as soon as one key is detected. I've had a closer look. read_char does indeed call out to lisp. I can't yet see any place where a non-local exit could cause a re-entry into read_key_sequence without re-initialising the static variables, but it might well be there. > > You have a point, here. Perhaps it would be better to get storage from > > the Emacs heap rather than using the stack. > I like using the stack, here, actually. I thought about that. It would mean passing this buffer as a parameter through lots of functions to get from where it's defined to where it gets filled. Here "lots" means a stack of, perhaps, 7 or 8 function calls. It would certainly be safer, though. > Maybe another option is to make raw_keybuf local to read_key_sequence, > and to *copy* it into the global raw_keybuf_buffer just before exiting. That's quite close to what I've done. The difficulty is in getting a pointer to that local buffer for use by the menu handling code. > Stefan > PS: Of course, even better would be to provide another way to get what > `this-single-command-raw-keys` returns, so we don't need to use a global > variable for it. E.g. have `read-key-sequence` return both the key > sequence and the raw key sequence. But we'd still have to support > `this-single-command-raw-keys` for the foreseeable future anyway. I don't think that idea would fly. Most uses of `read-key-sequence' aren't going to be interested in the raw events. -- Alan Mackenzie (Nuremberg, Germany).