[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r31428 - in trunk/subversion: include libsvn_subr

From: Daniel Shahaf <d.s_at_daniel.shahaf.co.il>
Date: Mon, 26 May 2008 12:50:12 +0300 (Jerusalem Daylight Time)

Arfrever Frehtes Taifersar Arahesis wrote on Mon, 26 May 2008 at 11:23 +0200:
> 2008-05-26 08:06 Daniel Shahaf <d.s_at_daniel.shahaf.co.il> napisa┼‚(a):
> > Karl Fogel wrote on Sun, 25 May 2008 at 22:30 -0400:
> >> Arfrever Frehtes Taifersar Arahesis <arfrever.fta_at_gmail.com> writes:
> >> >> Log:
> >> >> Implement the 'password-stores' config option.
> >> >>
> >> >> --- trunk/subversion/libsvn_subr/cmdline.c (r31427)
> >> >> +++ trunk/subversion/libsvn_subr/cmdline.c (r31428)
> >> >> @@ -435,17 +435,45 @@ svn_cmdline_setup_auth_baton(svn_auth_ba
> >> >>
> >> >> [...]
> >> >>
> >> >> #ifdef SVN_HAVE_KWALLET
> >> >> - if (get_auth_simple_provider(&provider, "kwallet", pool))
> >> >> - {
> >> >> - APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
> >> >> + if (apr_strnatcmp(password_store, "kwallet") == 0)
> >> >> + {
> >> >> + if (get_auth_simple_provider(&provider, "kwallet", pool))
> >> >> + {
> >> >> + APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
> >> >> + }
> >> >> + continue;
> >> >> + }
> >> >> +#endif
> >> >> + /* TODO: Error. */
> >
> > Don't you need to also allow the case where password_store == ""?
>
> svn_cstring_split() doesn't allow password_store to be "".
>

Sorry, I misread svn_cstring_split()'s docstring. It says that the
array will not contain empty elements -- I missed the "not".

> When the 'password-stores' config option is set to "", then
> password_stores->nelts should be 0, so the code inside the 'for (i =
> 0; i < password_stores->nelts; i++)' loop shouldn't be used.
>
> >> > Which error code should be used for invalid value of DAV-unrelated
> >> > configuration option?
> >>
> >
> > Nothing? It will fall back to plaintext passwords and get a warning
> > there...
>
> When the 'http-library' option is set to invalid value, then the
> "Invalid config: unknown HTTP library '%s'" error is printed instead
> of falling back to Neon.
> I would prefer consistent behavior.
>

That error is only printed when both serf and neon are enabled at
compile time. Therefore, ignoring gnome-keyring when it is not enabled
at compile time might be considered consistent...

e.g. If I have two clients, one with kwallet and one with both kwallet and
gnome-keyring, and my password-stores option is 'gnome-keyring,kwallet',
then I'll want the first client to use kwallet, not to complain that
'gnome-keyring' is not a known value...

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-26 11:50:29 CEST

This is an archived mail posted to the Subversion Dev mailing list.