GNU bug report logs - #78735
feature/igc: [PATCH] Reduce the size of the kbd-buffer GC root

Previous Next

Full log

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

From: Helmut Eller <eller.helmut <at> gmail.com>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 78735 <at> debbugs.gnu.org
Subject: Re: bug#78735: feature/igc: [PATCH] Reduce the size of the
 kbd-buffer GC root
Date: Tue, 10 Jun 2025 08:46:51 +0200

On Mon, Jun 09 2025, Pip Cet wrote:

[...]
>> +static union buffered_input_event *
>> +prev_kbd_event (union buffered_input_event *kbd_buffer,
>> +		union buffered_input_event *ptr)
>> +{
>> +  return ptr == kbd_buffer ? kbd_buffer + KBD_BUFFER_SIZE - 1 : ptr - 1;
>> +}
>> +
>> +static union buffered_input_event *
>> +next_kbd_event (union buffered_input_event *kbd_buffer,
>> +		union buffered_input_event *ptr)
>> +{
>> +  return ptr == kbd_buffer + KBD_BUFFER_SIZE - 1 ? kbd_buffer : ptr + 1;
>> +}
>
> Just out of curiosity, is there a reason for the extra kbd_buffer
> argument?  Since this code is replicated a few times, maybe it would be
> better to keep next_kbd_event in keyboard.c identical to the one in
> igc.c.  Maybe not, though...

The extra argument avoids accessing the global variable; that's all.

>> +  union buffered_input_event *fetch
>> +    = prev_kbd_event (kbd_buffer, kbd_fetch_ptr);
>> +  union buffered_input_event *store
>> +    = next_kbd_event (kbd_buffer, kbd_store_ptr);
>> +
>> +  if (fetch < store)
>> +    return scan_ambig (ss, fetch, store, closure);
>
> I think this will fail if kbd_store_ptr + 1 == kbd_fetch_ptr (i.e. the
> kbd_buffer is full)?

Hmm, indeed.

> if (fetch < store - 1)
>
> should work, though, and it's not like this case is common enough to
> worry about scanning the same event twice.

Yes, that should work.

Helmut

Previous Next

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