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

[PATCH] Enable error lifetime checking by default in maintainer mode

From: Malcolm Rowe <malcolm-svn-dev_at_farside.org.uk>
Date: 2007-01-07 15:58:25 CET

r22916 fixed two error leaks in the multiple-moves branch. I wouldn't
have noticed them at all except that I had noticed the SVN_DEBUG_ERROR
#define and decided to give it a go.

[For anyone who's not familiar with the macro: defining it causes a
tripwire function to be attached to the pool of every error we create.
If you handle the error and clear it with svn_error_clear(), nothing
happens, but if you forget (and leak the error), it fires at pool
cleanup time - when the process exits - and causes an abort().]

I'd like to enable this functionality by default (in maintainer mode) so
that these bugs are immediately obvious. Are there any objections to
committing the attached patch?

[[[
Enable error lifetime checking by default in maintainer mode, causing
processes to abort when terminating if any error objects have been
unhandled.

* subversion/libsvn_subr/error.c
  (err_abort, make_error_internal, svn_error_compose, svn_error_dup,
   svn_error_clear): Change code sections guarded by SVN_DEBUG_ERROR to
     use SVN_DEBUG instead.
]]]

Regards,
Malcolm

Index: subversion/libsvn_subr/error.c
===================================================================
--- subversion/libsvn_subr/error.c (revision 22915)
+++ subversion/libsvn_subr/error.c (working copy)
@@ -61,7 +61,7 @@ svn_error__locate(const char *file, long
 
 /* Cleanup function for errors. svn_error_clear () removes this so
    errors that are properly handled *don't* hit this code. */
-#if defined(SVN_DEBUG_ERROR)
+#if defined(SVN_DEBUG)
 static apr_status_t err_abort(void *data)
 {
   svn_error_t *err = data; /* For easy viewing in a debugger */
@@ -98,7 +98,7 @@ make_error_internal(apr_status_t apr_err
   new_error->line = error_line;
   /* XXX TODO: Unlock mutex here */
 
-#if defined(SVN_DEBUG_ERROR)
+#if defined(SVN_DEBUG)
   if (! child)
       apr_pool_cleanup_register(pool, new_error,
                                 err_abort,
@@ -199,7 +199,7 @@ svn_error_compose(svn_error_t *chain, sv
   while (chain->child)
     chain = chain->child;
 
-#if defined(SVN_DEBUG_ERROR)
+#if defined(SVN_DEBUG)
   /* Kill existing handler since the end of the chain is going to change */
   apr_pool_cleanup_kill(pool, chain, err_abort);
 #endif
@@ -213,14 +213,14 @@ svn_error_compose(svn_error_t *chain, sv
       if (chain->message)
         chain->message = apr_pstrdup(pool, new_err->message);
       chain->pool = pool;
-#if defined(SVN_DEBUG_ERROR)
+#if defined(SVN_DEBUG)
       if (! new_err->child)
         apr_pool_cleanup_kill(oldpool, new_err, err_abort);
 #endif
       new_err = new_err->child;
     }
 
-#if defined(SVN_DEBUG_ERROR)
+#if defined(SVN_DEBUG)
   apr_pool_cleanup_register(pool, chain,
                             err_abort,
                             apr_pool_cleanup_null);
@@ -272,7 +272,7 @@ svn_error_dup(svn_error_t *err)
         tmp_err->message = apr_pstrdup(pool, tmp_err->message);
     }
 
-#if defined(SVN_DEBUG_ERROR)
+#if defined(SVN_DEBUG)
   apr_pool_cleanup_register(pool, tmp_err,
                             err_abort,
                             apr_pool_cleanup_null);
@@ -286,7 +286,7 @@ svn_error_clear(svn_error_t *err)
 {
   if (err)
     {
-#if defined(SVN_DEBUG_ERROR)
+#if defined(SVN_DEBUG)
       while (err->child)
         err = err->child;
       apr_pool_cleanup_kill(err->pool, err, err_abort);

  • application/pgp-signature attachment: stored
Received on Sun Jan 7 15:58:37 2007

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