On 8/8/06, Vlad Georgescu <vgeorgescu@gmail.com> wrote:
> I modified this patch to use svn_atomic_init_once, and also made a
> couple of other changes.
>
> [[[
> Add client-side support for Cyrus SASL.
>
> * configure.in: Define SVN_HAVE_SASL.
>
> * subversion/libsvn_ra_svn/client.c
> (svn_ra_svn__init): Call svn_ra_svn__sasl_init.
>
> * subversion/libsvn_ra_svn/ra_svn_sasl.h: New file.
>
> * subversion/libsvn_ra_svn/sasl_auth.c: New file.
>
> * subversion/libsvn_ra_svn/simple_auth.c:
> Enclose content within an #ifndef SVN_HAVE_SASL .. #endif pair.
>
> * subversion/libsvn_ra_svn/ra_svn.h
> (SVN_RA_SVN__READBUF_SIZE,
> SVN_RA_SVN__WRITEBUF_SIZE): New defines.
> (svn_ra_svn_conn_st): Use them here.
> (svn_ra_svn__sasl_init): New declaration.
> ]]]
A few more problems:
+/* Define sane defaults for a sasl_security_properties_t structure.
+ See sasl.h for details. SASL needs to know our read buffer's size
+ when negotiating a security layer. */
+#define SVN_RA_SVN__DEFAULT_SECPROPS {0, 256, SVN_RA_SVN__READBUF_SIZE, \
+ 0, NULL, NULL}
Please explain what 256 means in this context. I think it's been
discussed on this list, but it really should make it into the comment.
+static void *sasl_mutex_alloc_cb(void)
+{
+ apr_thread_mutex_t *mutex;
+ apr_status_t apr_err;
+ if (apr_is_empty_array(free_mutexes))
+ {
+ apr_err = apr_thread_mutex_create(&mutex,
+ APR_THREAD_MUTEX_DEFAULT,
+ sasl_pool);
+ if (apr_err != APR_SUCCESS)
+ return NULL;
+ }
+ else
+ {
+ apr_err = apr_thread_mutex_lock(array_mutex);
+ if (apr_err != APR_SUCCESS)
+ return NULL;
+ mutex = *((apr_thread_mutex_t**)apr_array_pop(free_mutexes));
+ apr_err = apr_thread_mutex_unlock(array_mutex);
+ if (apr_err != APR_SUCCESS)
+ return NULL;
+ }
+ return mutex;
+}
You can't access the array without locking the mutex. The lock needs
to be pulled up outside the if statement, before you call
apr_is_empty_array, otherwise none of it is thread safe. Basically
you need to lock the mutex before ANY access to that array.
Other than that it seems fine to me.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 9 17:28:13 2006