Ben Collins-Sussman wrote:
>* svn_ra_dav_h
>
> In @brief, you say 'functions used by the server'. Huh? ra_dav is
> 100% client-side stuff.
>
whoops, copyage. Corrected (but still a bit brief for my tastes ;-))
> Please document the public declarations of the provider constructors
> more fully. How does it work? What runtime hash params does it
> use? How will it behave when invoked via the public svn_auth.h API?
> Look at svn_wc.h:svn_wc_get_simple_provider for an example.
>
> (For example, the server-cert provider doesn't actually provide
> server certs, just a flag; the doc string needs to make it really
> clear exactly what it does and how it works.)
>
done.
> Also, pleeeeeeze wrap to 80 columns. That's in the HACKING file. :-)
>
>
I changed the name of svn_ra_dav_get_ssl_client_password_file_provider
to svn_ra_dav_get_ssl_pw_file_provider to keep the line from wrapping
;-) The function name is still longer than 32 characters, but I don't
know a way around the naming scheme here
>* svn_auth.h
>
> (svn_auth_cred_client_ssl_t): ah, nice, you actually *did* pass
> back paths to the cert and key files. very nice. even though you
> don't have direct access to the binary key blob, this is almost as
> good. much clearer! In the doc string, make it clear that the file
> fields are *paths* to files.
>
done.
>* session.c
>
> Ack! 80 column overruns throughout your changes! Ack! Fix all
> those!!
>
done
> (server_ssl_callback): remove the // C++ comment please. :-)
>
>
done
> (client_ssl_keypw_callback, client_ssl_callback): whoa, tex... you
> never check the error return value from
> svn_auth_first_credentials(). Why not just wrap the call in the
> SVN_ERR() macro? And in client_ssl_callback, you're dereffing
> credentials when it may be NULL! Also, these funcs could use short
> docstrings.
>
We'll need to talk about this more - I cannot return a svn_error_t*
outright, and some of these providers simply cannot retry
>* config_file.c
>
> Your "ssl-ignore-*" examples are inverted from the doc strings.
> Look closely. :-)
>
>+ "### ssl-ignore-unknown-ca Allow untrusted server certificates\n"
>+ "### ssl-ignore-invalid-date Allow expired/postdated certificates\n"+ "### ssl-ignore-host-mismatch Allow certificates for other servers\n
>+ "# ignore-ssl-unknown-ca = true\n"
>+ "# ignore-ssl-host-mismatch = true\n"
>+ "# ignore-ssl-invalid-date = true\n"
>
whoops, done.
-David Waite
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Mar 11 16:55:10 2003