From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] master 5b5f441: read_key_sequence: correct the handling of raw_keybuf in recursive calls Date: Thu, 23 Nov 2017 09:35:03 -0500 Message-ID: References: <20171120181209.23553.97060@vcs0.savannah.gnu.org> <20171120181210.7946F20416@vcs0.savannah.gnu.org> <20171120195918.GB3917@ACM> <20171120203004.GC3917@ACM> <20171122192522.GA4288@ACM> <20171122210426.GB4288@ACM> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1511447617 9339 195.159.176.226 (23 Nov 2017 14:33:37 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 23 Nov 2017 14:33:37 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Alan Mackenzie Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Nov 23 15:33:26 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 1eHsZ6-0001VS-4m for ged-emacs-devel@m.gmane.org; Thu, 23 Nov 2017 15:33:20 +0100 Original-Received: from localhost ([::1]:44682 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eHsZD-0004ka-Bh for ged-emacs-devel@m.gmane.org; Thu, 23 Nov 2017 09:33:27 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:45457) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eHsZ7-0004kU-I8 for emacs-devel@gnu.org; Thu, 23 Nov 2017 09:33:22 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eHsZ4-0005ME-Tq for emacs-devel@gnu.org; Thu, 23 Nov 2017 09:33:21 -0500 Original-Received: from chene.dit.umontreal.ca ([132.204.246.20]:49087) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eHsZ4-0005LM-OX for emacs-devel@gnu.org; Thu, 23 Nov 2017 09:33:18 -0500 Original-Received: from lechazo.home (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.7/8.14.1) with ESMTP id vANEXGi1025347; Thu, 23 Nov 2017 09:33:17 -0500 Original-Received: by lechazo.home (Postfix, from userid 20848) id 945D260435; Thu, 23 Nov 2017 09:35:03 -0500 (EST) In-Reply-To: <20171122210426.GB4288@ACM> (Alan Mackenzie's message of "Wed, 22 Nov 2017 21:04:26 +0000") X-NAI-Spam-Flag: NO X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 0 X-NAI-Spam-Rules: 2 Rules triggered EDT_SA_DN_PASS=0, RV6165=0 X-NAI-Spam-Version: 2.3.0.9418 : core <6165> : inlines <6183> : streams <1771109> : uri <2538807> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 132.204.246.20 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:220394 Archived-At: > 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. Exactly: I can't convince myself that it's safe, even after spending some effort on it, so I think we need to makes its safety more obvious. > Which I think is what you're doing anyway. :-) Exactly. >> > 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. Ah, I see. I thought that by "getting a pointer to that local buffer" you meant that the menu handling code actually needs to get its hands at the raw_keybuf variable. Yes, I don't doubt that read-key-sequence can be called recursively, although I indeed hadn't thought about the above case. Here's another case where read-key-sequence can be invoked recursively: read_key_sequence calls keyremap_step which calls access_keymap_keyremap, which uses `call1` to run the Elisp code specified in the remapping keymap (e.g. input-decode-map for xterm-mouse-mode), which internally may decide to call read-key-sequence (currently at least xterm-mouse-mode and things like event-apply-control-modifier use read-event rather than read-key(-sequence), but there could be very valid reasons to use read-key(-sequence) in there). > 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. Hmm... that might work, but I still can't imagine what comment I could write next to the code to explain/prove that it's safe. I think a record_unwind_protect is the easiest way to make it safer. Yet, with the new concurrency feature, the unwind might reset the global var to a pointer into a stack area on another thread, which may have gotten stale in the mean time. Stefan