On Fri, Mar 08, 2013 at 08:10:23AM -0500, Vladimir Támara Patiño wrote: > > Following the advice of Stefan Sperling (see > http://marc.info/?l=openbsd-tech&m=136029207723238&w=4 ) > the attached patch is minimal to support collation in > OpenBSD-current it works with ISO-8859-1 and UTF-8 translatable > to ISO-8859-1. It includes: > * Program colldef to generate collation tables (for single-byte > encodings) > * Collation tables for spanish to be generated in > /usr/share/locale. * Implementation of strcoll, wcscoll, strxfrm and > wcsxfrm having into account that these functions are particular > cases of strcoll_l, wcscoll_l, strxfrm_l and wcsxfrm_l from xlocale > and that in near future possibly OpenBSD will implement xlocale > * Small changes to documentation * Regression test (test spanish > with ISO-8859-1 and specially UTF-8) > > The implementation is based on the one I sent before > ( http://marc.info/?l=openbsd-tech&m=136050321323182&w=4 and > http://marc.info/?l=openbsd-tech&m=136034304709498&w=4 ), that in > turn is based on FreeBSD implementation.
Thanks! This looks a bit better but is still too large for my taste. I'd like to concentrate on collation. To properly review your diff, I need to understand what needs to be added to OpenBSD' locale implementation. But I cannot clearly see that from reading your diff. There is still so much unrelated stuff in there. I'll cross-check other sources of information such as POSIX as additional references, of course. But first I need to understand what your new code really wants to do. It looks like your approach to adding collation support to OpenBSD is to take FreeBSD's code as a starting point and fudge that code into OpenBSD's libc. This approach reduces the quality of the code you end up with and makes it hard to review your changes. Instead, you should consider OpenBSD's code as your starting point. And add to that _only_ what needs to be done to support collation. Adding new functions like strxfrm_l() etc. just doesn't make sense to me. You shouldn't need to add any new functions. We already have strcoll() and strxfrm(). Those are interfaces from C89, so they're a bit dated. But they should be good enough for getting some basic collation support going. We can talk about adding support for newer interfaces later, but those should not matter for the initial implementation. If it helps, consider writing a new, clean, and small collation implementation for our libc, using C89 interfaces. Instead of porting over FreeBSD's implementation. If you can reuse bits from FreeBSD for your own implementation, fine (just don't forget to copy licence information). But adjust whatever FreeBSD code you use to fit our libc's model. Don't adjust our libc to fit FreeBSD's model. You have other unrelated changes in there, such as shuffling of MLINKS lists. Those add noise to the diff and make it harder to review.
