On Thu, 18 Nov 2010 15:16:49 +0100
Jakub Hrozek 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
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
