On Fri, Jul 02, 2010 at 10:29:47AM -0400, Dmitri Pal wrote:
Stephen Gallagher wrote:
> On 07/02/2010 09:37 AM, Dmitri Pal wrote:
> > Nack
>
> > You are leaking entry in success scenario.
> > I suggest a little bit cleaner approach:
>
> > while((ret = krb5_kt_next_entry(context, keytab, &entry, &cursor))
> > == 0){
> > krb5_unparse_name(context, entry.principal, &principal);
> > krb5_free_keytab_entry_contents(context, &entry);
> > if (strcmp(full_princ, principal) == 0) found = true;
> > free(principal);
> > if (found) break;
> > }
>
> Good catch. Fixed.
>
> > And I think that if you have explicit syslog messages in case of
> > failures it makes sense to have one also in the case krb5_kt_end_seq_get
> > fails.
>
>
> Sure. It should hopefully never happen, but added nonetheless.
>
>
> Thanks for the review! New patch attached.
>
Visual ack.
I would recommend to move the 'Verify the keytab' section into a
separate function and call it during the initialisation of the provider.
For the syslog messages I think it would make sense to create a new
utility function which does openlog(), syslog() and closelog(). This
would make it easier to send syslog messages for other parts of the code
and would help to add 'other logging APIs'.
bye,
Sumit
-------------------------
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you,
Dmitri Pal
Engineering Manager IPA project,
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel