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 19:01:27 UTC
svn commit: r1182984 -
/subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
Author: stefan2
Date: Thu Oct 13 17:01:27 2011
New Revision: 1182984
URL: http://svn.apache.org/viewvc?rev=1182984&view=rev
Log:
Simplify the code by using APR_HAS_THREADS directly as a parameter
instead of setting a thread_safe intermediate.
* subversion/tests/libsvn_subr/cache-test.c
(test_membuffer_cache_basic): simplify
Modified:
subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
Modified: subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/cache-test.c?rev=1182984&r1=1182983&r2=1182984&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/cache-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/cache-test.c Thu Oct 13 17:01:27 2011
@@ -182,16 +182,9 @@ test_membuffer_cache_basic(apr_pool_t *p
{
svn_cache__t *cache;
svn_membuffer_t *membuffer;
- svn_boolean_t thread_safe;
-
-#if APR_HAS_THREADS
- thread_safe = TRUE;
-#else
- thread_safe = FALSE;
-#endif
SVN_ERR(svn_cache__membuffer_cache_create(&membuffer, 10*1024, 1,
- thread_safe, pool));
+ APR_HAS_THREADS, pool));
/* Create a cache with just one entry. */
SVN_ERR(svn_cache__create_membuffer_cache(&cache,
Re: svn commit: r1182984 - /subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Sperling <st...@elego.de> writes:
> I also passed APR_HAS_THREADS as a boolean arg in some recent commits,
> inspired by danielsh's idea. But now I agree with philip that it's not
> good practice to do this. +1 on changing it.
We could add our own #define that we control.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1182984 -
/subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Oct 13, 2011 at 07:18:50PM +0100, Philip Martin wrote:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
> > My reading of
> > http://svn.apache.org/repos/asf/apr/apr/tags/0.9.0/include/apr.h.in
> > http://svn.apache.org/repos/asf/apr/apr/tags/0.9.0/include/apr.hnw
> > http://svn.apache.org/repos/asf/apr/apr/tags/0.9.0/include/apr.hw
> > is that APR_HAS_THREADS is always defined.
>
> Then we would be relying on APR to do it. It's not the way all
> preprocessor symbols are defined, for example svn_private_config.h
> generally uses <nothing> rather than #define XXX 0 and APR does the same
> for APR_IS_DEV_VERSION.
I also passed APR_HAS_THREADS as a boolean arg in some recent commits,
inspired by danielsh's idea. But now I agree with philip that it's not
good practice to do this. +1 on changing it.
Re: svn commit: r1182984 - /subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:
> Philip Martin wrote on Thu, Oct 13, 2011 at 18:44:44 +0100:
>> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>>
>> > First of all, I made the same patch yesterday elsewhere.
>>
>> Another bug :)
>>
>> > Second of all, the use is #if, not #ifdef, so I believe the macro is
>> > always defined (to 0 or 1).
>>
>> The one doesn't follow from the other. In either case the macro could
>> be zero, non-zero, no value or not defined:
>>
>> #if #ifdef
>> #define APR_HAS_THREADS 1 true true
>> #define APR_HAS_THREADS 0 false true
>> #define APR_HAS_THREADS true true
>> <nothing> false false
>>
>
> Is this standard behaviour?
I believe so.
>
>> So not defining APR_HAS_THREADS is a valid way to define no thread
>> support, but it means you can't use APR_HAS_THREADS directly as a
>> variable.
>>
>> It's possible that APR will always ensure that APR_HAS_THREADS is either
>> 0 or 1, but the C language does not.
>>
>
> My reading of
> http://svn.apache.org/repos/asf/apr/apr/tags/0.9.0/include/apr.h.in
> http://svn.apache.org/repos/asf/apr/apr/tags/0.9.0/include/apr.hnw
> http://svn.apache.org/repos/asf/apr/apr/tags/0.9.0/include/apr.hw
> is that APR_HAS_THREADS is always defined.
Then we would be relying on APR to do it. It's not the way all
preprocessor symbols are defined, for example svn_private_config.h
generally uses <nothing> rather than #define XXX 0 and APR does the same
for APR_IS_DEV_VERSION.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1182984 -
/subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Philip Martin wrote on Thu, Oct 13, 2011 at 18:44:44 +0100:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>
> > First of all, I made the same patch yesterday elsewhere.
>
> Another bug :)
>
> > Second of all, the use is #if, not #ifdef, so I believe the macro is
> > always defined (to 0 or 1).
>
> The one doesn't follow from the other. In either case the macro could
> be zero, non-zero, no value or not defined:
>
> #if #ifdef
> #define APR_HAS_THREADS 1 true true
> #define APR_HAS_THREADS 0 false true
> #define APR_HAS_THREADS true true
> <nothing> false false
>
Is this standard behaviour?
> So not defining APR_HAS_THREADS is a valid way to define no thread
> support, but it means you can't use APR_HAS_THREADS directly as a
> variable.
>
> It's possible that APR will always ensure that APR_HAS_THREADS is either
> 0 or 1, but the C language does not.
>
My reading of
http://svn.apache.org/repos/asf/apr/apr/tags/0.9.0/include/apr.h.in
http://svn.apache.org/repos/asf/apr/apr/tags/0.9.0/include/apr.hnw
http://svn.apache.org/repos/asf/apr/apr/tags/0.9.0/include/apr.hw
is that APR_HAS_THREADS is always defined.
> --
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com
Re: svn commit: r1182984 - /subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:
> First of all, I made the same patch yesterday elsewhere.
Another bug :)
> Second of all, the use is #if, not #ifdef, so I believe the macro is
> always defined (to 0 or 1).
The one doesn't follow from the other. In either case the macro could
be zero, non-zero, no value or not defined:
#if #ifdef
#define APR_HAS_THREADS 1 true true
#define APR_HAS_THREADS 0 false true
#define APR_HAS_THREADS true true
<nothing> false false
So not defining APR_HAS_THREADS is a valid way to define no thread
support, but it means you can't use APR_HAS_THREADS directly as a
variable.
It's possible that APR will always ensure that APR_HAS_THREADS is either
0 or 1, but the C language does not.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: svn commit: r1182984 -
/subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Philip Martin wrote on Thu, Oct 13, 2011 at 18:15:43 +0100:
> stefan2@apache.org writes:
>
> > Author: stefan2
> > Date: Thu Oct 13 17:01:27 2011
> > New Revision: 1182984
> >
> > URL: http://svn.apache.org/viewvc?rev=1182984&view=rev
> > Log:
> > Simplify the code by using APR_HAS_THREADS directly as a parameter
> > instead of setting a thread_safe intermediate.
> >
> > * subversion/tests/libsvn_subr/cache-test.c
> > (test_membuffer_cache_basic): simplify
> >
> > Modified:
> > subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
> >
> > Modified: subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/cache-test.c?rev=1182984&r1=1182983&r2=1182984&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/tests/libsvn_subr/cache-test.c (original)
> > +++ subversion/trunk/subversion/tests/libsvn_subr/cache-test.c Thu Oct 13 17:01:27 2011
> > @@ -182,16 +182,9 @@ test_membuffer_cache_basic(apr_pool_t *p
> > {
> > svn_cache__t *cache;
> > svn_membuffer_t *membuffer;
> > - svn_boolean_t thread_safe;
> > -
> > -#if APR_HAS_THREADS
> > - thread_safe = TRUE;
> > -#else
> > - thread_safe = FALSE;
> > -#endif
> >
> > SVN_ERR(svn_cache__membuffer_cache_create(&membuffer, 10*1024, 1,
> > - thread_safe, pool));
> > + APR_HAS_THREADS, pool));
>
> Dangerous. What are the valid values for APR_HAS_THREADS? When
> threading is present it uses something like:
>
> #define APR_HAS_THREADS 1
>
> but when threading is not present it may simply not define
> APR_HAS_THREADS at all.
>
First of all, I made the same patch yesterday elsewhere.
Second of all, the use is #if, not #ifdef, so I believe the macro is
always defined (to 0 or 1).
> --
> Philip
Re: svn commit: r1182984 - /subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
Posted by Philip Martin <ph...@wandisco.com>.
stefan2@apache.org writes:
> Author: stefan2
> Date: Thu Oct 13 17:01:27 2011
> New Revision: 1182984
>
> URL: http://svn.apache.org/viewvc?rev=1182984&view=rev
> Log:
> Simplify the code by using APR_HAS_THREADS directly as a parameter
> instead of setting a thread_safe intermediate.
>
> * subversion/tests/libsvn_subr/cache-test.c
> (test_membuffer_cache_basic): simplify
>
> Modified:
> subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
>
> Modified: subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/cache-test.c?rev=1182984&r1=1182983&r2=1182984&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/libsvn_subr/cache-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_subr/cache-test.c Thu Oct 13 17:01:27 2011
> @@ -182,16 +182,9 @@ test_membuffer_cache_basic(apr_pool_t *p
> {
> svn_cache__t *cache;
> svn_membuffer_t *membuffer;
> - svn_boolean_t thread_safe;
> -
> -#if APR_HAS_THREADS
> - thread_safe = TRUE;
> -#else
> - thread_safe = FALSE;
> -#endif
>
> SVN_ERR(svn_cache__membuffer_cache_create(&membuffer, 10*1024, 1,
> - thread_safe, pool));
> + APR_HAS_THREADS, pool));
Dangerous. What are the valid values for APR_HAS_THREADS? When
threading is present it uses something like:
#define APR_HAS_THREADS 1
but when threading is not present it may simply not define
APR_HAS_THREADS at all.
--
Philip