[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