On Mon, 2011-12-05 at 11:09 -0500, Simo Sorce wrote:
On Mon, 2011-12-05 at 11:06 -0500, Stephen Gallagher wrote:
> On Mon, 2011-12-05 at 09:30 -0500, Stephen Gallagher wrote:
> > On Fri, 2011-12-02 at 15:16 -0500, Simo Sorce wrote:
> > > ----- Original Message -----
> > > > Currently, SSSD only supports using libunistring to manage unicode
> > > > strings. There are some platforms out there (such as RHEL 5) that do
> > > > not
> > > > have libunistring available. With this patch, we add an optional
flag
> > > > to
> > > > autoconf to allow SSSD to link against Glib and use its unicode
> > > > functionality.
> > >
> > > Two quick comments.
> > > 1. Can you use a shorter function name than
sss_utf8_case_insensitive_equality ?
> > > I am all for clarity, but when the function name is so long arguments go
on the next line w/o any nesting it is bad :-)
> > >
> > > 2. given result returns just a boolean I would prefer to not use it and
just use ret.
> > > 0 is 'matches' and some other error code means it didn't
match, something like ERANGE could be used. or if you feel strongly about having your own
value define a value like -1 as ENOMATCH earlier in the code. It makes for a more usable
interface if you do not have to care about 2 return variables IMO.
> >
> >
> > Thanks for the review. Revised patch is attached.
> >
> > I opted to use ENOTUNIQ for the "not matched" response, since it
should
> > be impossible to receive that error from any of the underlying functions
> > within the comparison.
>
> Simo nacked off-list. I've added a new error code ENOMATCH (-1) and
> adjusted the #ifdef blocks to surround the functions completely (for
> readability).
>
> New patch attached.
Unfortunately I have no time to actually test the patch, but it's an ACK
from me on every other count.
I've been testing it locally since last week, along with the test tools
I created for verifying the original bugs around UTF8 support.
Pushed to master and sssd-1-6.
Backported for sssd-1-5 (makefile changes).