On Mon, Dec 06, 2010 at 09:25:01AM +0100, Jan Zelený wrote:
Sumit Bose <sbose(a)redhat.com> wrote:
> On Sat, Nov 20, 2010 at 12:25:02AM +0100, Jan Zeleny wrote:
> > After some complications I finally made the patch solving ticket #533
> > (different ccache files during multiple simultaneous logins of the same
> > user). It is based on Simo's idea to determine the ccache file name in
> > advance and count references to it.
> >
> > Jan
>
> I'm not sure if it is worth writing the reference counter and the new
> file to the cache. Having them in a hash should be sufficient and
> the reference counter does not have to be reseted at startup (which is
> missing).
>
> The reference counter is set in krb5_auth_send() but decremented in
> krb5_child_done(). This should be moved to krb5_auth_done() before any
> error checking to make sure that it is done under all circumstances.
>
> bye,
> Sumit
I'm sending updated patch. Both your objections were valid and they are now
resolved.
Thank you, it looks much better now, but I'm afraid I still have some
comments:
- please rebase to the current master
- I would prefer to have the ccache_names hash to be a member of
krb5_ctx instead of being a global variable
- please do not use hash_create() directly, but sss_hash_create(). There
are still some place which uses hash_create_ex(), but I think the should
be updated to use sss_hash_create() sooner or later, too.
- please use a suitable struct instead of the "ccname:ref_count" string
- it would be nice it you can add some helper functions to initialize
and increment the ref count value so that the value itself is actually
hidden from the caller
bye,
Sumit
Jan