[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Thread-safe error location in maintainer mode

From: Branko Čibej <brane_at_wandisco.com>
Date: Wed, 04 Mar 2015 13:44:25 +0100

On 04.03.2015 13:23, Philip Martin wrote:
> Branko Čibej <brane_at_wandisco.com> writes:
>
>> error stacks look sane. But, before committing the change, I'd like
>> someone else to review the patch because it's just a wee bit tricky in
>> places what with all the #ifdefs.
> Looks OK, a few minor things:
>
>> Index: subversion/libsvn_subr/error.c
>> ===================================================================
>> --- subversion/libsvn_subr/error.c (revision 1663941)
>> +++ subversion/libsvn_subr/error.c (working copy)
>> @@ -28,6 +28,10 @@
>> #include <apr_pools.h>
>> #include <apr_strings.h>
>>
>> +#if defined(SVN_DEBUG) && APR_HAS_THREADS
>> +#include <apr_thread_proc.h>
>> +#endif
> Why not an unconditional include?

I prefer to keep it conditional so that bugs in fiddling with the
#ifdefs lower down will cause a compiler error.

>> -/* XXX TODO: Define mutex here #if APR_HAS_THREADS */
>> +/* No-op destructor for apr_threadkey_private_create(). */
>> +static void null_threadkey_dtor(void *stuff) {}
>> +
>> +/* Init-once function for the thread-local keys.
>> + This function will never return an error. */
> It is easier for the reader if "svn_atomic__init_once" is explict rather
> than implied by "Init-once". If svn_atomic__init_once used a typedef
> for the callback that would be even better, but that is a separate
> change.

I'll fix the comment ... but a typedef for the svn_atomic__init_once
handler wouldn't really help, since you can't use that to define a function.

-- Brane
Received on 2015-03-04 13:44:58 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.