You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2015/03/03 21:28:30 UTC
svn commit: r1663780 - /subversion/trunk/subversion/include/svn_hash.h
Author: julianfoad
Date: Tue Mar 3 20:28:30 2015
New Revision: 1663780
URL: http://svn.apache.org/r1663780
Log:
* subversion/include/svn_hash.h
(svn_hash__gets): Remove #if from prototype to match definition.
Modified:
subversion/trunk/subversion/include/svn_hash.h
Modified: subversion/trunk/subversion/include/svn_hash.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_hash.h?rev=1663780&r1=1663779&r2=1663780&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_hash.h (original)
+++ subversion/trunk/subversion/include/svn_hash.h Tue Mar 3 20:28:30 2015
@@ -239,7 +239,6 @@ svn_hash_from_cstring_keys(apr_hash_t **
const apr_array_header_t *keys,
apr_pool_t *pool);
-#ifdef SVN_DEBUG
/* In debug builds, the svn_hash_gets macro forwards the parameters
* through this function in order to have parameter type checking,
* particularly for the key. The svn_hash_sets macro gets parameter
@@ -247,7 +246,6 @@ svn_hash_from_cstring_keys(apr_hash_t **
*/
void *
svn_hash__gets(apr_hash_t *ht, const char *key);
-#endif
/** Shortcut for apr_hash_get() with a const char * key.
*
Re: svn commit: r1663780 - /subversion/trunk/subversion/include/svn_hash.h
Posted by Julian Foad <ju...@btopenworld.com>.
All resolved via IRC now.
See r1663940.
- Julian
I (Julian Foad) wrote:
> Branko and Bert, I'm still not sure I understand what is needed for the
> Windows build.
[...]
> Now I don't know which is correct (#ifdef or not) and I understand I do need
> to add svn_hash__[gs]ets to a list of exports.
>
> Can you clarify further what still needs to be done, if anything, please?
>
> - Julian
Re: svn commit: r1663780 - /subversion/trunk/subversion/include/svn_hash.h
Posted by Julian Foad <ju...@btopenworld.com>.
Branko and Bert, I'm still not sure I understand what is needed for the Windows build.
>>> -#ifdef SVN_DEBUG
>>> /* In debug builds, the svn_hash_gets macro forwards the parameters
>>> * through this function in order to have parameter type checking,
>>> * particularly for the key. The svn_hash_sets macro gets parameter
>>> @@ -247,7 +246,6 @@ svn_hash_from_cstring_keys(apr_hash_t **
>>> */
>>> void *
>>> svn_hash__gets(apr_hash_t *ht, const char *key);
>>> -#endif
>>
> Branko Čibej wrote:
>> No, that's wrong. The should not be available in non-SVN_DEBUG mode
>> because nobody should be calling svn_hash__gets directly. All calls
>> should come from the expansion of the svn_hash_gets macro, which only
>> expands to the call in SVN_DEBUG mode.
>>
>> The problem with the "missing prototype" warning should be solved in the
>> same way we solve it for error tracing (see the definition of
>> SVN_ERR__TRACING in svn_error.h and how it's used in libsvn_subr/error.c).
> Bert Huijben wrote:
>> As noted on irc we have one .def file that is used for release and
>> maintenance builds. This generated file has all the function names
>> that are published by a .DLL.
>>
>> If the function is not in the list, we can’t use it in maintainer
>> builds from libraries != libsvn_subr.
We do want to use svn_hash__[sg]ets in maintainer builds from all libs, so you're saying I have to put it in that list. (Where is the list?)
>> And if it is in the list, it
>> must be available or the linker will return an error in release mode.
So its definition should not be inside #ifdef. OK, it's not.
Branko Čibej wrote:
> But the .def file generator doesn't look for #ifdef DEBUG blocks, right.
> Otherwise we'd have had this problem for ages with svn_err__locate
> (which, fwiw, is not declared within '#idfef DEBUG', as I implicitly
> stated [...]).
I can't find svn_error__trace mentioned inside any files in (root dir) or build/**.
The svn_error__trace definition is unconditional. (It has an #ifdef inside it, but that's not the main concern.)
The svn_error__trace declaration is inside #ifdef SVN_DEBUG (effectively; actually inside #ifdef SVN_ERR__TRACING which is defined iff SVN_DEBUG).
But Bert said yesterday[1] I need to remove the #ifdef from the declaration, at least that's how I understood it so I did so in r1663780.
Now I don't know which is correct (#ifdef or not) and I understand I do need to add svn_hash__[gs]ets to a list of exports.
Can you clarify further what still needs to be done, if anything, please?
- Julian
[1] http://colabti.org/irclogger/irclogger_log/svn-dev?date=2015-03-03#l77
Re: svn commit: r1663780 - /subversion/trunk/subversion/include/svn_hash.h
Posted by Branko Čibej <br...@wandisco.com>.
On 04.03.2015 09:37, Bert Huijben wrote:
> As noted on irc we have one .def file that is used for release and
> maintenance builds. This generated file has all the function names
> that are published by a .DLL.
>
> If the function is not in the list, we can’t use it in maintainer
> builds from libraries != libsvn_subr. And if it is in the list, it
> must be available or the linker will return an error in release mode.
But the .def file generator doesn't look for #ifdef DEBUG blocks, right.
Otherwise we'd have had this problem for ages with svn_err__locate
(which, fwiw, is not declared within '#idfef DEBUG', as I implicitly
stated below).
-- Brane
> *From:* Branko Čibej <ma...@wandisco.com>
> *Sent:* Wednesday, March 4, 2015 9:20 AM
> *To:* dev@subversion.apache.org <ma...@subversion.apache.org>
>
> On 03.03.2015 21:28, julianfoad@apache.org wrote:
> > Author: julianfoad
> > Date: Tue Mar 3 20:28:30 2015
> > New Revision: 1663780
> >
> > URL: http://svn.apache.org/r1663780
> > Log:
> > * subversion/include/svn_hash.h
> > (svn_hash__gets): Remove #if from prototype to match definition.
> >
> > Modified:
> > subversion/trunk/subversion/include/svn_hash.h
> >
> > Modified: subversion/trunk/subversion/include/svn_hash.h
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_hash.h?rev=1663780&r1=1663779&r2=1663780&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/include/svn_hash.h (original)
> > +++ subversion/trunk/subversion/include/svn_hash.h Tue Mar 3
> 20:28:30 2015
> > @@ -239,7 +239,6 @@ svn_hash_from_cstring_keys(apr_hash_t **
> > const apr_array_header_t *keys,
> > apr_pool_t *pool);
> >
> > -#ifdef SVN_DEBUG
> > /* In debug builds, the svn_hash_gets macro forwards the parameters
> > * through this function in order to have parameter type checking,
> > * particularly for the key. The svn_hash_sets macro gets parameter
> > @@ -247,7 +246,6 @@ svn_hash_from_cstring_keys(apr_hash_t **
> > */
> > void *
> > svn_hash__gets(apr_hash_t *ht, const char *key);
> > -#endif
>
> No, that's wrong. The should not be available in non-SVN_DEBUG mode
> because nobody should be calling svn_hash__gets directly. All calls
> should come from the expansion of the svn_hash_gets macro, which only
> expands to the call in SVN_DEBUG mode.
>
> The problem with the "missing prototype" warning should be solved in the
> same way we solve it for error tracing (see the definition of
> SVN_ERR__TRACING in svn_error.h and how it's used in libsvn_subr/error.c).
>
> -- Brane
>
Re: svn commit: r1663780 - /subversion/trunk/subversion/include/svn_hash.h
Posted by Bert Huijben <be...@qqmail.nl>.
As noted on irc we have one .def file that is used for release and maintenance builds. This generated file has all the function names that are published by a .DLL.
If the function is not in the list, we can’t use it in maintainer builds from libraries != libsvn_subr. And if it is in the list, it must be available or the linker will return an error in release mode.
Bert
Sent from Windows Mail
From: Branko Čibej
Sent: Wednesday, March 4, 2015 9:20 AM
To: dev@subversion.apache.org
On 03.03.2015 21:28, julianfoad@apache.org wrote:
> Author: julianfoad
> Date: Tue Mar 3 20:28:30 2015
> New Revision: 1663780
>
> URL: http://svn.apache.org/r1663780
> Log:
> * subversion/include/svn_hash.h
> (svn_hash__gets): Remove #if from prototype to match definition.
>
> Modified:
> subversion/trunk/subversion/include/svn_hash.h
>
> Modified: subversion/trunk/subversion/include/svn_hash.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_hash.h?rev=1663780&r1=1663779&r2=1663780&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_hash.h (original)
> +++ subversion/trunk/subversion/include/svn_hash.h Tue Mar 3 20:28:30 2015
> @@ -239,7 +239,6 @@ svn_hash_from_cstring_keys(apr_hash_t **
> const apr_array_header_t *keys,
> apr_pool_t *pool);
>
> -#ifdef SVN_DEBUG
> /* In debug builds, the svn_hash_gets macro forwards the parameters
> * through this function in order to have parameter type checking,
> * particularly for the key. The svn_hash_sets macro gets parameter
> @@ -247,7 +246,6 @@ svn_hash_from_cstring_keys(apr_hash_t **
> */
> void *
> svn_hash__gets(apr_hash_t *ht, const char *key);
> -#endif
No, that's wrong. The should not be available in non-SVN_DEBUG mode
because nobody should be calling svn_hash__gets directly. All calls
should come from the expansion of the svn_hash_gets macro, which only
expands to the call in SVN_DEBUG mode.
The problem with the "missing prototype" warning should be solved in the
same way we solve it for error tracing (see the definition of
SVN_ERR__TRACING in svn_error.h and how it's used in libsvn_subr/error.c).
-- Brane
Re: svn commit: r1663780 - /subversion/trunk/subversion/include/svn_hash.h
Posted by Branko Čibej <br...@wandisco.com>.
On 03.03.2015 21:28, julianfoad@apache.org wrote:
> Author: julianfoad
> Date: Tue Mar 3 20:28:30 2015
> New Revision: 1663780
>
> URL: http://svn.apache.org/r1663780
> Log:
> * subversion/include/svn_hash.h
> (svn_hash__gets): Remove #if from prototype to match definition.
>
> Modified:
> subversion/trunk/subversion/include/svn_hash.h
>
> Modified: subversion/trunk/subversion/include/svn_hash.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_hash.h?rev=1663780&r1=1663779&r2=1663780&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_hash.h (original)
> +++ subversion/trunk/subversion/include/svn_hash.h Tue Mar 3 20:28:30 2015
> @@ -239,7 +239,6 @@ svn_hash_from_cstring_keys(apr_hash_t **
> const apr_array_header_t *keys,
> apr_pool_t *pool);
>
> -#ifdef SVN_DEBUG
> /* In debug builds, the svn_hash_gets macro forwards the parameters
> * through this function in order to have parameter type checking,
> * particularly for the key. The svn_hash_sets macro gets parameter
> @@ -247,7 +246,6 @@ svn_hash_from_cstring_keys(apr_hash_t **
> */
> void *
> svn_hash__gets(apr_hash_t *ht, const char *key);
> -#endif
No, that's wrong. The should not be available in non-SVN_DEBUG mode
because nobody should be calling svn_hash__gets directly. All calls
should come from the expansion of the svn_hash_gets macro, which only
expands to the call in SVN_DEBUG mode.
The problem with the "missing prototype" warning should be solved in the
same way we solve it for error tracing (see the definition of
SVN_ERR__TRACING in svn_error.h and how it's used in libsvn_subr/error.c).
-- Brane