Re: [Emacs-diffs] master 5b5f441: read_key_sequence: correct the handling of raw_keybuf in recursive calls

[Alan Mackenzie <acm@muc.de>]

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).