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/05 00:01:56 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:
> +++ subversion/branches/svn_mutex/subversion/libsvn_subr/svn_mutex.c Sat Jul 2 10:20:56 2011
> @@ -27,19 +27,19 @@
> /* 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;
> + svn_mutex__t **mutex = data;
> + *mutex = NULL;
>
Is this change is correct? Looking at other pool cleanup handlers (eg
the ones in libsvn_subr/error.c), their void * argument is an
'svn_error_t *', not a pointer-to-pointer...
> return APR_SUCCESS;
> }
Re: svn commit: r1142191 - in
/subversion/branches/svn_mutex/subversion: include/private/ libsvn_fs/
libsvn_fs_fs/ libsvn_subr/
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Yeah, in reading the code I see everything is consistent.
r1147314 should make this confusing less likely in the future.
Stefan Fuhrmann wrote on Fri, Jul 15, 2011 at 22:19:37 +0200:
> On 05.07.2011 00:01, Daniel Shahaf wrote:
> >stefan2@apache.org wrote on Sat, Jul 02, 2011 at 10:20:57 -0000:
> >>+++ subversion/branches/svn_mutex/subversion/libsvn_subr/svn_mutex.c Sat Jul 2 10:20:56 2011
> >>@@ -27,19 +27,19 @@
> >> /* 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;
> >>+ svn_mutex__t **mutex = data;
> >>+ *mutex = NULL;
> >>
> >Is this change is correct? Looking at other pool cleanup handlers (eg
> >the ones in libsvn_subr/error.c), their void * argument is an
> >'svn_error_t *', not a pointer-to-pointer...
> Yes, this is "unusual" but correct. We provide
> MUTEX as parameter to the cleanup, which is
> a double pointer (while apr_mutex is only an
> ordinary pointer).
>
> The example you cite (make_error_internal)
> nowhere uses double indirection.
>
> -- Stefan^2.
Re: svn commit: r1142191 - in /subversion/branches/svn_mutex/subversion:
include/private/ libsvn_fs/ libsvn_fs_fs/ libsvn_subr/
Posted by Stefan Fuhrmann <st...@alice-dsl.de>.
On 05.07.2011 00:01, Daniel Shahaf wrote:
> stefan2@apache.org wrote on Sat, Jul 02, 2011 at 10:20:57 -0000:
>> +++ subversion/branches/svn_mutex/subversion/libsvn_subr/svn_mutex.c Sat Jul 2 10:20:56 2011
>> @@ -27,19 +27,19 @@
>> /* 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;
>> + svn_mutex__t **mutex = data;
>> + *mutex = NULL;
>>
> Is this change is correct? Looking at other pool cleanup handlers (eg
> the ones in libsvn_subr/error.c), their void * argument is an
> 'svn_error_t *', not a pointer-to-pointer...
Yes, this is "unusual" but correct. We provide
MUTEX as parameter to the cleanup, which is
a double pointer (while apr_mutex is only an
ordinary pointer).
The example you cite (make_error_internal)
nowhere uses double indirection.
-- Stefan^2.