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 <stork...@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
> @@ -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;
> @@ -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 */
> @@ -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

Reply via email to