all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Stefan Monnier <monnier@IRO.UMontreal.CA>
To: Alan Mackenzie <acm@muc.de>
Cc: emacs-devel@gnu.org
Subject: Re: [Emacs-diffs] master 5b5f441: read_key_sequence: correct the handling of raw_keybuf in recursive calls
Date: Wed, 22 Nov 2017 15:29:00 -0500	[thread overview]
Message-ID: <jwvbmjunk3w.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <20171122192522.GA4288@ACM> (Alan Mackenzie's message of "Wed, 22 Nov 2017 19:25:22 +0000")

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

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

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


        Stefan



  reply	other threads:[~2017-11-22 20:29 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20171120181209.23553.97060@vcs0.savannah.gnu.org>
     [not found] ` <20171120181210.7946F20416@vcs0.savannah.gnu.org>
2017-11-20 18:41   ` [Emacs-diffs] master 5b5f441: read_key_sequence: correct the handling of raw_keybuf in recursive calls Stefan Monnier
2017-11-20 19:59     ` Alan Mackenzie
2017-11-20 20:09       ` Stefan Monnier
2017-11-20 20:30         ` Alan Mackenzie
2017-11-20 21:43           ` Stefan Monnier
2017-11-22 19:25             ` Alan Mackenzie
2017-11-22 20:29               ` Stefan Monnier [this message]
2017-11-22 21:04                 ` Alan Mackenzie
2017-11-23 14:35                   ` Stefan Monnier
2017-11-23 18:10                     ` Alan Mackenzie
2017-11-23 18:46                       ` Stefan Monnier
2017-11-28 18:03                         ` Alan Mackenzie
2017-11-28 20:30                           ` Eli Zaretskii
2017-12-01 16:48                             ` Alan Mackenzie
2017-12-01 18:17                               ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=jwvbmjunk3w.fsf-monnier+emacs@gnu.org \
    --to=monnier@iro.umontreal.ca \
    --cc=acm@muc.de \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.