This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][BZ #16009] fix memory handling in strxfrm_l


+		/* 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]