Re: [hackers] [st][PATCH] fix: realloc kpress buffer as needed.

From: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
Date: Wed, 14 May 2025 00:36:48 +0200

On Tue, May 13, 2025 at 10:18:45PM +0200, Storkman wrote:
> On Tue, May 13, 2025 at 10:56:23AM +0000, NRK wrote:
> > On Mon, May 12, 2025 at 01:53:08AM +0900, yahei wrote:
> > > If XBufferOverflow occuers, the client should recall the function with
> > > the same event and a buffer of adequate size to obtain the string.
> >
> > Do you have any actual usecase where the current buffer size is a
> > problem? If yes, I think it's better to increase the buffer size to
> > something more reasonable if possible than to mess around with static
> > variables.
> >
> > - NRK
> >
>
> Patch didn't attach correctly to my previous message.
>
> --
> Storkman

> From 4268fe94eaa4d56ed2b4fc5bf0c3d846de52f296 Mon Sep 17 00:00:00 2001
> From: Paul Storkman <storkman_AT_storkman.nl>
> Date: Tue, 13 May 2025 16:54:35 +0200
> Subject: [PATCH] Temporarily allocate a larger input buffer if needed
>
> ---
> x.c | 30 +++++++++++++++++++-----------
> 1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/x.c b/x.c
> index d73152b..e9f7287 100644
> --- a/x.c
> +++ b/x.c
> _AT_@ -1843,7 +1843,7 @@ kpress(XEvent *ev)
> {
> XKeyEvent *e = &ev->xkey;
> KeySym ksym = NoSymbol;
> - char buf[64], *customkey;
> + char buf[64], *customkey, *str = buf;
> int len;
> Rune c;
> Status status;
> _AT_@ -1854,23 +1854,28 @@ kpress(XEvent *ev)
>
> if (xw.ime.xic) {
> len = XmbLookupString(xw.ime.xic, e, buf, sizeof buf, &ksym, &status);
> - if (status == XBufferOverflow)
> - return;
> + if (status == XBufferOverflow) {
> + str = xmalloc(len);
> + len = XmbLookupString(xw.ime.xic, e, str, len, &ksym, &status);
> + if (status == XBufferOverflow)
> + goto FINISH;
> + }
> } else {
> len = XLookupString(e, buf, sizeof buf, &ksym, NULL);
> }
> +
> /* 1. shortcuts */
> for (bp = shortcuts; bp < shortcuts + LEN(shortcuts); bp++) {
> if (ksym == bp->keysym && match(bp->mod, e->state)) {
> bp->func(&(bp->arg));
> - return;
> + goto FINISH;
> }
> }
>
> /* 2. custom keys from config.h */
> if ((customkey = kmap(ksym, e->state))) {
> ttywrite(customkey, strlen(customkey), 1);
> - return;
> + goto FINISH;
> }
>
> /* 3. composed string from input method */
> _AT_@ -1878,17 +1883,20 @@ kpress(XEvent *ev)
> return;
> if (len == 1 && e->state & Mod1Mask) {
> if (IS_SET(MODE_8BIT)) {
> - if (*buf < 0177) {
> - c = *buf | 0x80;
> - len = utf8encode(c, buf);
> + if (*str < 0177) {
> + c = *str | 0x80;
> + len = utf8encode(c, str);
> }
> } else {
> - buf[1] = buf[0];
> - buf[0] = '\033';
> + str[1] = str[0];
> + str[0] = '\033';
> len = 2;
> }
> }
> - ttywrite(buf, len, 1);
> + ttywrite(str, len, 1);
> +FINISH:
> + if (str != buf)
> + free(str);
> }

I don't like this pattern of str != buf when str can be a static buffer in some
cases. Probably better to assign to a separate variable if allocated.
Something like:

char *alloc = NULL;
if (something) {
        alloc = malloc(len);
        str = alloc;
}
...
free(alloc); Note that free(NULL) is fine.

>
> void
> --
> 2.45.2
>

BTW I do not use IME myself. So testing and detailed reports should be done by
others.

-- 
Kind regards,
Hiltjo
Received on Wed May 14 2025 - 00:36:48 CEST

This archive was generated by hypermail 2.3.0 : Wed May 14 2025 - 00:48:38 CEST