You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2011/10/13 18:58:56 UTC
svn propchange: r1182979 - svn:log
Author: stefan2
Revision: 1182979
Modified property: svn:log
Modified: svn:log at Thu Oct 13 16:58:56 2011
------------------------------------------------------------------------------
--- svn:log (original)
+++ svn:log Thu Oct 13 16:58:56 2011
@@ -1,2 +1,6 @@
+Remove a #if APR_HAS_THREADS statement. They have been one reason for
+introducing svn_mutex__t in the first place.
+
* subversion/libsvn_fs/fs-loader.c
- (svn_fs_initialize): use APR_HAS_THREADS as function parameter instead of #if state
+ (svn_fs_initialize): use APR_HAS_THREADS as function parameter instead
+ of #if statements
Re: svn propchange: r1182979 - svn:log
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Oct 14, 2011 at 10:11:53AM +0200, Stefan Fuhrmann wrote:
> The current code operates along two dimensions:
> * provide the same interface whether or not
> APR provides synchronization structures
> * allow for the application to decide _dynamically_
> whether it is actually necessary to create and
> use a mutex (see svn_cache__t API for instance).
>
> If APR_HAS_THREADS is 0 and the application
> insists on enabling a mutex, svn_mutex__init() will
> return an error.
>
> So, maybe we should redefine the parameter to
> mean, "_if_ I am a multi-threaded application, do I
> need a mutex here". Then it could always be set
> to TRUE or FALSE and the APR_HAS_THREADS
> setting would only be evaluated within *__init().
>
> I prepared a patch that does just that and fixes up
> all callers as well.
Sounds good to me. Thanks!
Re: svn propchange: r1182979 - svn:log
Posted by Stefan Fuhrmann <eq...@web.de>.
On 13.10.2011 22:36, Stefan Sperling wrote:
> On Thu, Oct 13, 2011 at 04:58:56PM -0000, stefan2@apache.org wrote:
>> Author: stefan2
>> Revision: 1182979
>> Modified property: svn:log
>>
>> Modified: svn:log at Thu Oct 13 16:58:56 2011
>> ------------------------------------------------------------------------------
>> --- svn:log (original)
>> +++ svn:log Thu Oct 13 16:58:56 2011
>> @@ -1,2 +1,6 @@
>> +Remove a #if APR_HAS_THREADS statement. They have been one reason for
>> +introducing svn_mutex__t in the first place.
> So you're saying that svn_mutex__t is intended to provide an abstraction
> layer which hides APR_HAS_THREADS? If so, why does svn_mutex__init()
> take a boolean parameter 'enable_mutex', which essentially specifies whether
> APR supports threads? Shouldn't the function figure that out by itself?
The current code operates along two dimensions:
* provide the same interface whether or not
APR provides synchronization structures
* allow for the application to decide _dynamically_
whether it is actually necessary to create and
use a mutex (see svn_cache__t API for instance).
If APR_HAS_THREADS is 0 and the application
insists on enabling a mutex, svn_mutex__init() will
return an error.
So, maybe we should redefine the parameter to
mean, "_if_ I am a multi-threaded application, do I
need a mutex here". Then it could always be set
to TRUE or FALSE and the APR_HAS_THREADS
setting would only be evaluated within *__init().
I prepared a patch that does just that and fixes up
all callers as well.
-- Stefan^2.
Re: svn propchange: r1182979 - svn:log
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Oct 13, 2011 at 04:58:56PM -0000, stefan2@apache.org wrote:
> Author: stefan2
> Revision: 1182979
> Modified property: svn:log
>
> Modified: svn:log at Thu Oct 13 16:58:56 2011
> ------------------------------------------------------------------------------
> --- svn:log (original)
> +++ svn:log Thu Oct 13 16:58:56 2011
> @@ -1,2 +1,6 @@
> +Remove a #if APR_HAS_THREADS statement. They have been one reason for
> +introducing svn_mutex__t in the first place.
So you're saying that svn_mutex__t is intended to provide an abstraction
layer which hides APR_HAS_THREADS? If so, why does svn_mutex__init()
take a boolean parameter 'enable_mutex', which essentially specifies whether
APR supports threads? Shouldn't the function figure that out by itself?