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 21:04:26 +0000 Message-ID: <20171122210426.GB4288@ACM> References: <20171120181209.23553.97060@vcs0.savannah.gnu.org> <20171120181210.7946F20416@vcs0.savannah.gnu.org> <20171120195918.GB3917@ACM> <20171120203004.GC3917@ACM> <20171122192522.GA4288@ACM> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1511384970 23703 195.159.176.226 (22 Nov 2017 21:09:30 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 22 Nov 2017 21:09:30 +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 22:09:25 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 1eHcGm-0005QM-C6 for ged-emacs-devel@m.gmane.org; Wed, 22 Nov 2017 22:09:20 +0100 Original-Received: from localhost ([::1]:41329 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eHcGr-0007Jv-RQ for ged-emacs-devel@m.gmane.org; Wed, 22 Nov 2017 16:09:25 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:35676) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eHcFe-0007IC-49 for emacs-devel@gnu.org; Wed, 22 Nov 2017 16:08:11 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eHcFa-0005hY-Ge for emacs-devel@gnu.org; Wed, 22 Nov 2017 16:08:10 -0500 Original-Received: from colin.muc.de ([193.149.48.1]:21320 helo=mail.muc.de) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1eHcFa-0005gy-6E for emacs-devel@gnu.org; Wed, 22 Nov 2017 16:08:06 -0500 Original-Received: (qmail 51086 invoked by uid 3782); 22 Nov 2017 21:08:04 -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 22:08:04 +0100 Original-Received: (qmail 6052 invoked by uid 1000); 22 Nov 2017 21:04:26 -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:220371 Archived-At: Hello again, Stefan. On Wed, Nov 22, 2017 at 15:29:00 -0500, Stefan Monnier wrote: > >> 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. > I was thinking more of: > (progn (catch 'foo (read-key-sequence ...)) > ... > (this-single-command-raw-keys)) > where a timer throws `foo` during read-key-sequence. > Since this-single-command-raw-keys uses the raw_keybuf, it ends up > dereferencing the "stale" pointer to the var that was local in > read_key_sequence. I'm still not sure there's a problem. But I think the effort involved in showing there's no problem would exceed that needed to solve it, so we might as well just assume there's a problem. Which I think is what you're doing anyway. :-) > >> > 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. > No, I mean, I like the fact that your code uses the stack. > >> 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. > Hmm... could you show me that menu handling code you're referring to? > The only user I can see of raw_keybuf (outside of local uses within > read_key_sequence) is this-single-command-raw-keys, so maybe I'm > missing something. read_menu_command calls read_key_sequence. It is called by this stack of functions: - read_key_sequence is called by - read_menu_command .. ...... .. - read_menu_input - tty_menu_activate - tty_menu_show, which is one value of terminal->menu_show_hook. This is called in its turn by: - Fx_popup_menu, which besides being called from many places in lisp is also called from - read_char_x_menu_prompt - read_char - read_key_sequence This is the recursion which makes it difficult just to use a global buffer for the raw key events. How about this idea? Split Fx_popup_menu into two pieces, Fx_popup_menu which just initialises the global raw key buffer then calls sub_x_popup_menu. read_char_x_menu_prompt (see above list) would then call sub_x_popup_menu to avoid emptying the global buffer. Other than that, the global raw key buffer would be initialised in command_loop_1 and (possibly) read_key_sequence_vs. This way, everything which appends to the raw key buffer could just use the global variables, without any dangerous shenanigans on the stack. > >> 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. > [ This is really a side-discussion about a better solution for the > long-term, but we need to solve the current problem regardless. ] > I was thinking of adding a new function read-raw-key-sequence which > returns both the normal and the raw key sequences, with the hope that > all users of this-single-command-raw-keys could eventually be changed to > make use of that new function instead. Ah, I see. Yes, that would be a good idea. There are only a few uses of this-single-command-raw-keys (six) in the Emacs sources. Any external uses will have been written by experts, who would probably be willing to convert to a more rational interface, such as read-raw-key-sequence. > Stefan -- Alan Mackenzie (Nuremberg, Germany).