-1
There are some problems in the .c portion of this patch. See below.
On Sat, Jan 26, 2002 at 03:54:45AM +0100, Sander Striker wrote:
>...
> +#if APR_POOL_DEBUG
> +#define svn_pool_create(p) svn_pool_create_debug(p, APR_POOL__FILE_LINE__)
> +#endif /* APR_POOL_DEBUG */
>...
> +#if APR_POOL_DEBUG
> +#define svn_pool_clear(p) svn_pool_clear_debug(p, APR_POOL__FILE_LINE__)
> +#endif /* APR_POOL_DEBUG */
It might be nice to group these together, with a comment explaining what is
going on.
>...
> +++ ./subversion/libsvn_subr/svn_error.c Fri Jan 25 15:12:39 2002
>...
> +#undef svn_pool_clear
> +void
> +svn_pool_clear (apr_pool_t *p);
> +#endif
That should be: #endif /* APR_POOL_DEBUG */
> -#ifndef SVN_POOL_DEBUG
> +#if !APR_POOL_DEBUG
> apr_pool_t *
> svn_pool_create (apr_pool_t *parent_pool)
> -#else /* SVN_POOL_DEBUG */
> -apr_pool_t *
> -svn_pool_create_debug (apr_pool_t *parent_pool,
> - const char *file,
> - int line)
> -#endif /* SVN_POOL_DEBUG */
> {
> apr_pool_t *ret_pool;
>
> apr_pool_create_ex (&ret_pool, parent_pool, abort_on_pool_failure,
> APR_POOL_FDEFAULT);
> +#else /* APR_POOL_DEBUG */
> +apr_pool_t *
> +svn_pool_create_debug (apr_pool_t *parent_pool,
> + const char *file_line)
> +{
> + apr_pool_t *ret_pool;
> +
> + apr_pool_create_ex_debug (&ret_pool, parent_pool, abort_on_pool_failure,
> + APR_POOL_FDEFAULT, file_line);
> +#endif /* APR_POOL_DEBUG */
The _debug forms are supposed to be always-defined. Thus, the #if stuff
around these sections should not be present.
>...
> -#ifndef SVN_POOL_DEBUG
> +#if !APR_POOL_DEBUG
> void
> svn_pool_clear (apr_pool_t *p)
> -#else /* SVN_POOL_DEBUG */
> +#else /* APR_POOL_DEBUG */
> void
> svn_pool_clear_debug (apr_pool_t *p,
> - const char *file,
> - int line)
> -#endif /* SVN_POOL_DEBUG */
> + const char *file_line)
> +#endif /* APR_POOL_DEBUG */
Same here.
>...
> +#if !APR_POOL_DEBUG
> +apr_pool_t *
> +svn_pool_create_debug (apr_pool_t *parent_pool,
> + const char *file_line)
woah. Here is another svn_pool_create_debug ??
Looking through the rest of this patch, it isn't ready to go in. There
appear to be multiple definitions of the different symbols, and the #if
logic is overly complex / not-required.
The functions should always be defined. The _debug forms can always call the
APR _debug forms. The non-debug forms can be #ifdef'd to call the APR debug
forms with something like "svn:<undefined>" for the file/line argument. That
will allow things that haven't been recompiled to at least be marked as
coming from Subversion.
Please update appropriate and resubmit to the list. Thanks!
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:37:00 2006