[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Client-side support for Cyrus SASL

From: Vlad Georgescu <vgeorgescu_at_gmail.com>
Date: 2006-06-16 16:36:25 CEST

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

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.