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. Simo. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
