Author: stefan2
Date: Sat Jul 2 00:17:51 2011
New Revision: 1142129
URL: http://svn.apache.org/viewvc?rev=1142129&view=rev
Log:
Incorporate feedback on the svn_mutex API documentation.
Also, improve the initialization code to cover more usage
scenarios that I encountered when switching to the new API.
* subversion/include/private/svn_mutex.h
(svn_mutex__t): describe the "untypical" usage pattern and its rationale
(svn_mutex__init, svn_mutex__unlock): greatly improve docstrings
* subversion/libsvn_subr/svn_mutex.c
(uninit): new function
(svn_mutex__init): handle concurrent initialization; register cleanup
Modified:
subversion/branches/svn_mutex/subversion/include/private/svn_mutex.h
subversion/branches/svn_mutex/subversion/libsvn_subr/svn_mutex.c
Modified: subversion/branches/svn_mutex/subversion/include/private/svn_mutex.h
URL:
http://svn.apache.org/viewvc/subversion/branches/svn_mutex/subversion/include/private/svn_mutex.h?rev=1142129&r1=1142128&r2=1142129&view=diff
==============================================================================
--- subversion/branches/svn_mutex/subversion/include/private/svn_mutex.h
(original)
+++ subversion/branches/svn_mutex/subversion/include/private/svn_mutex.h Sat
Jul 2 00:17:51 2011
@@ -37,6 +37,11 @@ extern "C" {
/**
* This is a simple wrapper around @c apr_thread_mutex_t and will be a
* valid identifier even if APR does not support threading.
+ *
+ * @note In contrast to other structures, this one shall be treated as
+ * a pointer type, i.e. be instantiated and be passed by value instead by
+ * reference. There is simply no point in introducing yet another level
+ * of indirection and pointers to check for validity.
*/
typedef struct svn_mutex__t
{
@@ -51,10 +56,13 @@ typedef struct svn_mutex__t
#endif
} svn_mutex__t;
-/** Initialize the @a *mutex with a lifetime defined by @a pool, if
- * @a enable_mutex is TRUE and with @c NULL otherwise. If @a enable_mutex
- * is set but threading is not supported by APR, this function returns an
- * @c APR_ENOTIMPL error.
+/** Initialize the @a *mutex. If @a enable_mutex is TRUE, the mutex will
+ * actually be created with a lifetime defined by @a pool. Otherwise, the
+ * wrapped pointer will be set to @c NULL and @ref svn_mutex__lock as well
+ * as @ref svn_mutex__unlock will be no-ops.
+ *
+ * If @a enable_mutex is set but threading is not supported by APR, this
+ * function returns an @c APR_ENOTIMPL error.
*/
svn_error_t *
svn_mutex__init(svn_mutex__t *mutex,
@@ -70,6 +78,13 @@ svn_mutex__lock(svn_mutex__t mutex);
/** Release the @a mutex, previously acquired using @ref svn_mutex__lock
* that has been enabled in @ref svn_mutex__init.
+ *
+ * Since this is often used as part of the calling function's exit
+ * sequence, we accept that function's current return code in @a err.
+ * If it is not @ref SVN_NO_ERROR, it will be used as the return value -
+ * irrespective of the possible internal failures during unlock. If @a err
+ * is @ref SVN_NO_ERROR, internal failures of this function will be
+ * reported in the return value.
*/
svn_error_t *
svn_mutex__unlock(svn_mutex__t mutex,
Modified: subversion/branches/svn_mutex/subversion/libsvn_subr/svn_mutex.c
URL:
http://svn.apache.org/viewvc/subversion/branches/svn_mutex/subversion/libsvn_subr/svn_mutex.c?rev=1142129&r1=1142128&r2=1142129&view=diff
==============================================================================
--- subversion/branches/svn_mutex/subversion/libsvn_subr/svn_mutex.c (original)
+++ subversion/branches/svn_mutex/subversion/libsvn_subr/svn_mutex.c Sat Jul 2
00:17:51 2011
@@ -24,6 +24,15 @@
#include "svn_private_config.h"
#include "private/svn_mutex.h"
+/* Destructor to be called as part of the pool cleanup procedure. */
+static apr_status_t uninit(void *data)
+{
+ svn_mutex__t *mutex = data;
+ mutex->mutex = NULL;
+
+ return APR_SUCCESS;
+}
+
svn_error_t *
svn_mutex__init(svn_mutex__t *mutex,
svn_boolean_t enable_mutex,
@@ -33,12 +42,16 @@ svn_mutex__init(svn_mutex__t *mutex,
mutex->mutex = NULL;
if (enable_mutex)
{
+ apr_thread_mutex_t *apr_mutex;
apr_status_t status =
- apr_thread_mutex_create(&mutex->mutex,
+ apr_thread_mutex_create(&apr_mutex,
APR_THREAD_MUTEX_DEFAULT,
pool);
if (status)
return svn_error_wrap_apr(status, _("Can't create mutex"));
+
+ mutex->mutex = apr_mutex;
+ apr_pool_cleanup_register(pool, mutex, uninit, apr_pool_cleanup_null);
}
#else
if (enable_mutex)