On Thu, 18 Nov 2010 15:16:49 +0100 Jakub Hrozek <[email protected]> wrote:
> On Mon, Nov 08, 2010 at 10:14:18PM +0100, Jakub Hrozek wrote: > > [PATCH 1/6] Common include file for SLAPI plugin logging > > Consolidate the common logging macros into common/util.h and use > > them in SLAPI plugins instead of calling slapi_log_error() directly. > > > > https://fedorahosted.org/freeipa/ticket/408 NACK, reintroduces a define of a function that was private to 389ds that we stoppped using. Everything else look fine > > [PATCH 2/6] Stricter compilation flags > > Use a little stricter compilation flags, in particular -Wall and > > treat implicit function declarations as errors. ACK > > [PATCH 3/6] Use internal implementation of internal Kerberos > > functions Don't use KRB5_PRIVATE. > > > > The patch implements and uses the following krb5 functions that are > > otherwise private in recent MIT Kerberos releases: > > * krb5_principal2salt_norealm > > * krb5_free_ktypes NACK, i we re-implement functions we should use our own namespace or we risk conflicts with the real functions in the library. > > [PATCH 4/6] Don't use deprecated ldap_bind_s > > ldap_bind_s is marked as deprecated in new libldap releases. ACK > > [PATCH 5/6] Silence compilation warnings in SLAPI plugins > > The most important part of the patch is exporting hexbuf() in > > ipapwd.h Also uses strcasecmp() instead of PL_strcasecmp() since we > > were not including nspr headers and linking against it - I hope > > this is OK, we can revert if we need to be portable to platforms > > with no strcasecmp(). The rest are cosmetic fixes. It seems to me that hexbuf() is used only in ipapwd_encoding.c, what about moving it there and making it static instead ? Adding the header would make less changes than replacing PL_strcasecmp everywhere. Any reason why replacing is easier than just adding the proper header ? > > [PATCH 6/6] ipa-client code cleanup > > Fixes errors about implicit function declaration and moves > > duplicated gettext code into a common module. Also silences some > > warnings. ACK > > Patches 3 - 6 fix https://fedorahosted.org/freeipa/ticket/454 > > Attached are patches rebased on top of current master, esp. the UUID > patch. Only needs minor changes, mostly a very good set, thanks! Simo. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
