Re: Review of dont-* branch

On Thu, May 01, 2008 at 07:53:11PM +0300, Daniel Shahaf wrote:
> Stefan Sperling wrote on Thu, 1 May 2008 at 17:28 +0200:
> > On Tue, Apr 29, 2008 at 08:29:46PM +0300, Daniel Shahaf wrote:
> > > > Index: subversion/libsvn_subr/simple_providers.c
> > > > ===================================================================
> > > > --- subversion/libsvn_subr/simple_providers.c (.../trunk) (revision 30801)
> > > > +++ subversion/libsvn_subr/simple_providers.c (.../branches/dont-save-plaintext-passwords-by-default) (revision 30836)
> > > > @@ -258,46 +272,117 @@ simple_save_creds_helper(svn_boolean_t *saved,
> > > > + /* Don't store passwords in any form if the user has told
> > > > + * us not to do so. */
> > > > + if (! dont_store_passwords)
> > > > {
> > > > + svn_boolean_t may_save_password = FALSE;
> > > >
> > > > + /* If the password is going to be stored encrypted, go right
> > > > + * ahead and store it to disk. Else determine whether saving
> > > > + * in plaintext is OK. */
> > > > + if (strcmp(passtype, SVN_AUTH__WINCRYPT_PASSWORD_TYPE) == 0
> > > > + || strcmp(passtype, SVN_AUTH__KEYCHAIN_PASSWORD_TYPE) == 0)
> > >
> > > Do we want to hardcode which password types are encrypted?
> >
> > Well, what's the alternative?
> >
>
> /me discovers that the PASSWORD_TYPE constants are private to simple_providers.c
>
> Document this? If someone adds a password type and greps for
> SIMPLE_PASSWORD_TYPE (try this), they won't know that somewhere in the file
> there is a list of "special" password types that SIMPLE isn't one of them...

I'm not sure how exactly you want me to document this.
Can you provide a patch?

-- 
Stefan Sperling <stsp_at_elego.de>                    Software Monkey
 
German law requires the following banner :(
elego Software Solutions GmbH                            HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 
13355 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                               CEO: Olaf Wagner
 
Store password unencrypted (yes/no)? No