Re: [PATCH] Cache ssl client cert passphrase in gnome-keyring

On Wed, May 28, 2008 at 06:13:37PM +0530, Senthil Kumaran S wrote:
> >> +/* Save passphrase for a client certificate in auth/ cache */
> >> +svn_error_t *
> >> +svn_auth__ssl_client_cert_pw_file_save_creds_helper
> >> + (svn_boolean_t *saved,
> >> + void *credentials,
> >> + void *provider_baton,
> >> + apr_hash_t *parameters,
> >> + const char *realmstring,
> >> + svn_auth__password_set_t passphrase_set,
> >> + const char *passtype,
> >> + apr_pool_t *pool)
> >> +{
> >
> > This function duplicates a lot of code. Would it be feasible to merge it
> > with the function it is based on?
>
> The original function handles the caching of username and password, but here we
> do only passphrase caching. Yes, it duplicates code, but it is difficult to
> identify whether we are using this function for SVN_AUTH_CRED_SIMPLE or
> SVN_AUTH_CRED_SSL_CLIENT_CERT_PW since it is hardcoded in the function. I haven
> changed it in the updated patch.

OK, in this case we can just leave it as is.
 
> >> +/* This implements 'svn_auth_plaintext_passphrase_prompt_func_t'. */
> >> +svn_error_t *
> >> +svn_cmdline_auth_plaintext_passphrase_prompt(svn_boolean_t *may_save_plaintext,
> >> + const char *realmstring,
> >> + void *baton,
> >> + apr_pool_t *pool)
> >> +{
> >
> > This one also duplicates some code.
> > Could we merge this function with the one it is based on?
> > The merged function could take more parameters than the original (e.g.
> > the banner above the prompt could be a new parameter).
>
> Yes this is merged now.

Great, thank you! Looks very good now.

I would recommend that you post your howto about setting up a server
and clients which use encrypted certificates to this list again. Because
without this information, your patch cannot be tested properly.

I have not gotten around to following your howto. I don't think I will
have the time to do this soon -- too much stuff on my todo list :-/
As such, I cannot promise that I can test the patch by running it.
I hope someone else can find the time to do this.

Stefan