2008/5/26 Daniel Shahaf <d.s_at_daniel.shahaf.co.il>:
> 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...
OK.
I will replace:
<snip>
#ifdef SVN_HAVE_GNOME_KEYRING
if (apr_strnatcmp(password_store, "gnome-keyring") == 0)
{
if (get_auth_simple_provider(&provider, "gnome_keyring", pool))
{
APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *)
= provider;
}
continue;
}
#endif
</snip>
With:
<snip>
if (apr_strnatcmp(password_store, "gnome-keyring") == 0)
{
#ifdef SVN_HAVE_GNOME_KEYRING
if (get_auth_simple_provider(&provider, "gnome_keyring", pool))
{
APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *)
= provider;
}
#endif
continue;
}
</snip>
(And the same for KWallet-related code.)
Received on 2008-05-26 12:29:18 CEST