On 6/15/06, Garrett Rooney <rooneg@electricjellyfish.net> wrote:
> On 6/15/06, Vlad Georgescu <vgeorgescu@gmail.com> wrote:
> > I created two new files, trivial_auth.c and sasl_auth.c, both of which
> > export svn_ra_svn__auth. The contents of these files are surrounded
> > with #ifdefs, so only one of them will be compiled, depending on
> > whether or not SVN_HAVE_SASL is defined.
> >
> > I haven't done anything on the server side yet, so all this does right
> > now is authenticate using the mechanisms that svnserve currently
> > supports (ANONYMOUS, EXTERNAL, CRAM-MD5). In theory, all the other
> > mechanisms should automagically work when I add full SASL support to
> > svnserve.
> >
> > Please test/review this and report any problems.
>
> First off, nice work. This looks like you're making some great
> progress towards integrating SASL into our system.
>
> A few comments based on a quick read of the diff:
>
> svn_ra_svn__auth isn't a very meaningful name. Maybe svn_ra_svn__handle_auth?
Okay.
> Your initialization of sasl is scoped to a pool, but I'm not sure
> that's correct. The sasl lib appears to use global data, and thus
> should be initialized once at startup, and once at shutdown. There
> should be a static variable that indicates if we've already
> initialized sasl (assuming that it's not just ok to call its init
> function repeatadly), and sasl_done() should be called from an atexit
> handler, not a pool cleanup handler.
Done.
> Your patch introduces numerous lines that are longer than 80 columns,
> those should be broken at 80 columns, as specified in hacking.html.
Fixed.
> There are also several places where your indenting is off:
Fixed.
[[[
Implement support for Cyrus SASL on the client side.
* configure.in
Define SVN_HAVE_SASL
* subversion/libsvn_ra_svn/sasl_auth.c: New file.
* subversion/libsvn_ra_svn/trivial_auth.c: New file. Contains
functions taken from client.c.
* subversion/libsvn_ra_svn/client.c
(ra_svn_session_baton_t): Moved to ra_svn.h and renamed to
svn_ra_svn__session_baton_t. Changed all occurances to reflect this.
(find_mech, read_success): Moved to trivial_auth.c.
(do_auth): Moved to trivial_auth.c and renamed to svn_ra_svn__handle_auth.
(auth_response): Renamed to svn_ra_svn__auth_response.
(svn_ra_svn__init): Call svn_ra_svn__sasl_init.
(open_session): Call svn_ra_svn__sasl_new.
Updated comments to reflect changes.
* subversion/libsvn_ra_svn/ra_svn.h
Conditionally #include <sasl/sasl.h>
(svn_ra_svn_conn_st): Add new member: sasl_conn.
(svn_ra_svn__handle_auth,
svn_ra_svn__auth_response,
svn_ra_svn__sasl_init,
svn_ra_svn__sasl_new): New declarations.
]]]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jun 16 16:48:58 2006