You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2011/07/02 22:40:43 UTC

Re: svn commit: r1142191 - in /subversion/branches/svn_mutex/subversion: include/private/ libsvn_fs/ libsvn_fs_fs/ libsvn_subr/

stefan2@apache.org wrote on Sat, Jul 02, 2011 at 10:20:57 -0000:
> Author: stefan2
> Date: Sat Jul  2 10:20:56 2011
> New Revision: 1142191
> 
> URL: http://svn.apache.org/viewvc?rev=1142191&view=rev
> Log:
> Bring svn_mutex__* API more in line with its APR counterpart.
> 
> * subversion/include/private/svn_mutex.h
>   (svn_mutex__t): make it an actual typedef instead of a wrapper struct
>   (svn_mutex__init, svn_mutex__lock, svn_mutex__unlock): switch to pointer style API
> * subversion/libsvn_subr/svn_mutex.c
>   (uninit, svn_mutex__lock, svn_mutex__unlock): adapt implementation to API change
>   (svn_mutex__init): dito; use SVN error code instead of APR_ENOIMPL
...
> +++ subversion/branches/svn_mutex/subversion/include/private/svn_mutex.h Sat Jul  2 10:20:56 2011
> @@ -28,6 +28,7 @@
> @@ -37,35 +38,34 @@ 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
> -{
>  #if APR_HAS_THREADS
>  
> -  /** A mutex for synchronization between threads. It may be NULL, in
> -   * which case no synchronization will take place. The latter is useful,
> -   * if implement some functionality where synchronization is optional.
> -   */
> -  apr_thread_mutex_t *mutex;
> -  
> +/** A mutex for synchronization between threads. It may be NULL, in
> + * which case no synchronization will take place. The latter is useful,
> + * if implement some functionality where synchronization is optional.
> + */
> +typedef apr_thread_mutex_t svn_mutex__t;
> +
> +#else
> +
> +/** Dummy definition. The content will never be actually accessed.
> + */
> +typedef int svn_mutex__t;

s/int/void/ ?

(to cause a compile-time error if it's ever dereferenced)


> +
>  #endif
> -} svn_mutex__t;

Re: svn commit: r1142191 - in /subversion/branches/svn_mutex/subversion: include/private/ libsvn_fs/ libsvn_fs_fs/ libsvn_subr/

Posted by Stefan Fuhrmann <eq...@web.de>.
On 02.07.2011 22:40, Daniel Shahaf wrote:
> stefan2@apache.org wrote on Sat, Jul 02, 2011 at 10:20:57 -0000:
>> Author: stefan2
>> Date: Sat Jul  2 10:20:56 2011
>> New Revision: 1142191
>>
>> URL: http://svn.apache.org/viewvc?rev=1142191&view=rev
>> Log:
>> Bring svn_mutex__* API more in line with its APR counterpart.
>>
>> * subversion/include/private/svn_mutex.h
>>    (svn_mutex__t): make it an actual typedef instead of a wrapper struct
>>    (svn_mutex__init, svn_mutex__lock, svn_mutex__unlock): switch to pointer style API
>> * subversion/libsvn_subr/svn_mutex.c
>>    (uninit, svn_mutex__lock, svn_mutex__unlock): adapt implementation to API change
>>    (svn_mutex__init): dito; use SVN error code instead of APR_ENOIMPL
> ...
>> +++ subversion/branches/svn_mutex/subversion/include/private/svn_mutex.h Sat Jul  2 10:20:56 2011
>> @@ -28,6 +28,7 @@
>> @@ -37,35 +38,34 @@ 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
>> -{
>>   #if APR_HAS_THREADS
>>
>> -  /** A mutex for synchronization between threads. It may be NULL, in
>> -   * which case no synchronization will take place. The latter is useful,
>> -   * if implement some functionality where synchronization is optional.
>> -   */
>> -  apr_thread_mutex_t *mutex;
>> -
>> +/** A mutex for synchronization between threads. It may be NULL, in
>> + * which case no synchronization will take place. The latter is useful,
>> + * if implement some functionality where synchronization is optional.
>> + */
>> +typedef apr_thread_mutex_t svn_mutex__t;
>> +
>> +#else
>> +
>> +/** Dummy definition. The content will never be actually accessed.
>> + */
>> +typedef int svn_mutex__t;
> s/int/void/ ?
>
> (to cause a compile-time error if it's ever dereferenced)
>
Done in r1142810. Thanks for the review.

-- Stefan^2.