[PATCH][BZ #16009] fix memory handling in strxfrm_l
Leonhard Holz
leonhard.holz@web.de
Wed Dec 31 09:38:00 GMT 2014
>> + /* Remember weight index and length of backward sequence. */
>> + stack_push (backw, weight_idx, len);
>
> This looks dangerous; you could overflow the stack with it if you have
> enough backward sequences. You'll be better off doing what the
> strcoll uncached algorithm does, i.e. traverse all backward sequences
> and then from back to front, get the weights of each sequence again.
> It is slower, but won't need the additional space.
Ok. This needs some calculations for offsets that will not make the code
easier to read but maybe we can refactor it later.
>> + /* This is a little optimization: many collation specifications have
>> + a `position' rule at the end and if no non-ignored character
>> + is found the last \1 byte is immediately followed by a \0 byte
>> + signalling this. We can avoid the \1 byte(s). */
>
> Split out this optimization into a separate patch.
This is copied from the cached path so it needs to be there to reproduce
the functionality.
> If you're going the route of defining a fixed size, you'll have to go
> through the pain of proving that the size you've chosen is optimal.
> Instead, just use malloc to allocate the required size and if it
> fails, fall back to the non-cached version. In that case, your test
> case changes are no longer valid and you'll need to verify by manually
> disabling the cached path to test the non-cached paths.
>
I want to object here. The needed cache size can be in a region where
the malloc will not fail but hurt the system performance considerable
(swapping), especially as the malloced size is five times the string
size. I think that low level functions like strxfrm should not carry
such risks.
Leonhard
More information about the Libc-alpha
mailing list