On 2022-10-24 10:01 AM, NRK wrote:
> On Mon, Oct 24, 2022 at 09:21:37AM +0000, Andy Gozas wrote:
>> • XmbLookupString leaves the ksym unchanged if not filled and
>> XLookupString
>> [probably] sets it to NoSymbol (that's what XLookupKeysym does, but
>> whether
>> or not XLookupString shares this behavior is unclear [1]), so we can
>> just
>> set it to NoSymbol in the beginning ourselves and check if it was
>> changed
>> later
>
> Initializing ksym to `NoSymbol` seems like a good idea.
>
>> • Since we can actually get the whole composed text when using
>> XmbLookupString by reallocating the buffer, I think we should do that
>> — why
>> stop at 512 bytes?
>
> Mainly because using I think that the dynamic allocation patch made the
> control flow of the function more complicated than necessary.
> Backward goto is pretty bad in specific.
>
> But if you _do_ want to dynamically allocate, you only need to allocate
> right before buffer is being used.
>
> But which approach to take is the maintainer's call, not mine.
> I've attched both fixed-size and dynamic-allocation patch (but
> simplified without goto).
>
> - NRK
Yeah, I think your dynamic patch might be a bit better when it comes to
readability, only thing I noticed is that it is missing initialization
of ksym to NoSymbol.
---
Andy Gozas.
Received on Mon Oct 24 2022 - 12:44:58 CEST