GNU bug report logs - #78943
feature/igc [PATCH] Trace current minor maps exactly

Previous Next

Full log

Message #17 received at 78943 <at> debbugs.gnu.org (full text, mbox):

From: Helmut Eller <eller.helmut <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: pipcet <at> protonmail.com, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 78943 <at> debbugs.gnu.org
Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly
Date: Sat, 12 Jul 2025 11:05:36 +0200

On Sat, Jul 12 2025, Eli Zaretskii wrote:

>> Cc: 78943 <at> debbugs.gnu.org
>> From: Helmut Eller <eller.helmut <at> gmail.com>
>> Date: Fri, 11 Jul 2025 23:10:15 +0200
>> 
>> A simple fix for Fminor_mode_key_binding would be to copy the modes and
>> maps arrays to an alloca'd area before calling Flookup_key.
>> 
>> In the patch below (for master) I tried something different: remove the
>> global variables cmm_maps and cmm_modes entirely.
>> 
>> Assuming that there are rarely more than a handful of minor modes
>> active, we can allocate the temporary arrays on the stack; for the case
>> with very many minor modes we use the GC heap.
>
> We usually don't use the stack for any data larger than 16KB.  Does
> this case fit that limit?

Yes: sizeof (struct cmm_vector) == 288.

>> The patch uses the GC heap if there are more than 16 minor mode keymaps
>> active.  I'm not sure if that is enough; viper mode already activates 8.
>> 
>> A nice aspect of the patch is that Fcurrent_active_maps no longer uses
>> the temparary arrays: the code in the patch knows how to build a list in
>> reverse order.  So with the patch, current_minor_maps is called less
>> frequently.  The most frequent callers are now tool_bar_items and
>> tab_bar_items which call current_minor_maps when the user scrolls
>> around.
>
> Thanks, but please also explain why we should install something like
> this on master and not on the igc branch, since the discussion started
> from the latter.

The main reason is that Pip said: "We should really fix this on master
and just use vectors..."

> What advantages will this get us on master?

On master, current_minor_maps returns a pointer to a buffer.  That
pointer is safe to use only until the next call to current_minor_maps.
This constraint is easy to forget: Fminor_mode_key_binding uses the
pointer for longer than that.

(Fminor_mode_key_binding calls Flookup_key which can execute arbitrary
Lisp code and hence could call current_minor_maps while the pointer to
the buffer is still used.).

With the patch, this bug would be fixed; that's only a tiny advantage
because in practice the problem occurs extremely rarely.

A more important advantage would be that it reduces "mental load"
because the new current_minor_maps is easier to use.

Helmut

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.