On Thu, Feb 07, 2013 at 05:16:37PM +0100, Bert Huijben wrote:
> > I agree that we shouldn't segfault. But it seems you've changed
> > the default behaviour with this commit, from 'ask' to 'yes',
> > in case the option isn't specified. See below.
> >
> > > @@ -372,8 +372,9 @@ svn_auth__simple_creds_cache_set(svn_boo
> > > simple_provider_baton_t *b =
> > > (simple_provider_baton_t *)provider_baton;
> > >
> > > - if (svn_cstring_casecmp(store_plaintext_passwords,
> > > - SVN_CONFIG_ASK) == 0)
> > > + if (store_plaintext_passwords
> > > + && svn_cstring_casecmp(store_plaintext_passwords,
> > > + SVN_CONFIG_ASK) == 0)
> >
> > I think this should be:
> >
> > if (!store_plaintext_passwords
> > || svn_cstring_casecmp(store_plaintext_passwords,
> > SVN_CONFIG_ASK) == 0)
> >
> > to make 'ask' the default catch-all case.
>
> Which makes things much harder to verify,
Verify what and why?
> and will eventually fall back to
> allowing it anyway, because with the pre-1.6 api there is no way to 'ask'.
>
> All our initializers set this value one way or another, so I didn't change a
> default. Or every svn invocation would have crashed since your change.
> (See r1443562 for a reproduction recipe)
Yeah, the default 'svn' behaviour hasn't changed, of course.
I still think it makes sense to write the code in accordance with the
intended default, even if the default ends up deciding to store the
password since no conflict prompt callback was provided by old API users.
Received on 2013-02-07 17:48:47 CET