Re: [PATCH v3 0/1] MR7766: win32u: Preserve result string from multiple WINE_IME_POST_UPDATE call during ImeProcessKey.

[Byeongsik Jeon (＠bsjeon)]

On Tue Apr  8 12:49:49 2025 +0000, Rémi Bernon wrote:
> I think this loses the previous result if it was != NULL but the new
> update has result_str == NULL. What about:
>     struct imm_thread_data *data = get_imm_thread_data();
>     UINT id = -1, comp_len, result_len, prev_result_len;
>     WCHAR *prev_result_str, *tmp;
>     struct ime_update *update;
>     TRACE( "hwnd %p, cursor_pos %u, comp_str %s, result_str %s\n", hwnd, cursor_pos,
>            debugstr_w(comp_str), debugstr_w(result_str) );
>     comp_len = comp_str ? wcslen( comp_str ) + 1 : 0;
>     result_len = result_str ? wcslen( result_str ) + 1 : 0;
>     /* prepend or keep the previous result string, if there was any */
>     if (!data->ime_process_vkey || !data->update) prev_result_str = NULL;
>     else prev_result_str = data->update->result_str;
>     prev_result_len = prev_result_str ? wcslen( prev_result_str ) + 1 : 0;
>     if (!prev_result_len && !result_len) tmp = NULL;
>     else if (!(tmp = malloc( (prev_result_len + result_len) *
> sizeof(WCHAR) ))) return;
>     if (prev_result_len && result_len) prev_result_len -= 1; /* concat
> both strings */
>     if (prev_result_len) memcpy( tmp, prev_result_str, prev_result_len *
> sizeof(WCHAR) );
>     if (result_len) memcpy( tmp + prev_result_len, result_str,
> result_len * sizeof(WCHAR) );
>     result_len += prev_result_len;
>     result_str = tmp;
>     if (!(update = malloc( offsetof(struct ime_update, buffer[comp_len +
> result_len]) ))) return;
>     update->cursor_pos = cursor_pos;
>     update->comp_str = comp_str ? memcpy( update->buffer, comp_str,
> comp_len * sizeof(WCHAR) ) : NULL;
>     update->result_str = result_str ? memcpy( update->buffer + comp_len,
> result_str, result_len * sizeof(WCHAR) ) : NULL;
>     if (!(update->vkey = data->ime_process_vkey))
>     {
>         pthread_mutex_lock( &imm_mutex );
>         id = update->scan = ++ime_update_count;
>         update->vkey = VK_PROCESSKEY;
>         list_add_tail( &ime_updates, &update->entry );
>         pthread_mutex_unlock( &imm_mutex );
>         NtUserPostMessage( hwnd, WM_WINE_IME_NOTIFY,
> IMN_WINE_SET_COMP_STRING, id );
>     }
>     else
>     {
>         update->scan = data->ime_process_scan;
>         free( data->update );
>         data->update = update;
>     }
>     free( result_str );

Yes. That's where the previous ime_update calls are ignored.
This is fine for comp_str, but it's problematic for result_str.

The change you suggest is similar to my personal early version. v1 removed the malloc addition and compacted the code, but now that I look at it, it seems too implicit, so I've updated it with the code you suggest. I changed a small memory freeing problem.

Also, v1 had a meaningless dead code.