Re: Detecting unhandled errors

"C. Michael Pilato" <cmpilato@collab.net> writes:

> "Sander Striker" <striker@apache.org> writes:
>
>> Ah, right!
>>
>> So, would this do the trick? Is this desired, or should the caller clear
>> the error itself?
>
> Feels (to me) more correct for the consumers to explicitly clear the
> errors they are finished with.

Not sure which is correct, after all the function does have "handle"
in it's name. If we do clear the error then it should be documented
in svn_error.h.

I think I have identified all the fs-test error leaks, they are all in
fs-test.c itself. One thing I noticed is that when an program creates
and clears lots of errors it can be very hard to determine which error
got missed. I am using a local patch to register the error itself as
the cleanup data pointer. What do you think?

Index: subversion/libsvn_subr/error.c
===================================================================
--- subversion/libsvn_subr/error.c (revision 7398)
+++ subversion/libsvn_subr/error.c (working copy)
@@ -66,7 +66,9 @@
 #if defined(SVN_DEBUG_ERROR)
 static apr_status_t err_abort (void *data)
 {
+ svn_error_t *err = data;
   abort();
+ err = NULL;
 }
 #endif
 
@@ -85,9 +87,6 @@
     {
       if (apr_pool_create (&pool, NULL))
         abort ();
-#if defined(SVN_DEBUG_ERROR)
- apr_pool_cleanup_register(pool, NULL, err_abort, NULL);
-#endif
     }
 
   /* Create the new error structure */
@@ -101,6 +100,11 @@
   new_error->line = error_line;
   /* XXX TODO: Unlock mutex here */
 
+#if defined(SVN_DEBUG_ERROR)
+ if (! child)
+ apr_pool_cleanup_register(pool, new_error, err_abort, NULL);
+#endif
+
   return new_error;
 }
 
@@ -161,6 +165,10 @@
   while (chain->child)
     chain = chain->child;
 
+#if defined(SVN_DEBUG_ERROR)
+ apr_pool_cleanup_kill (pool, chain, err_abort);
+#endif
+
   /* Copy the new error chain into the old chain's pool. */
   while (new_err)
     {
@@ -169,13 +177,18 @@
       *chain = *new_err;
       chain->message = apr_pstrdup (pool, new_err->message);
       chain->pool = pool;
+#if defined(SVN_DEBUG_ERROR)
+ if (! new_err->child)
+ apr_pool_cleanup_kill (oldpool, new_err, err_abort);
+#endif
       new_err = new_err->child;
     }
 
- /* Destroy the new error chain. */
 #if defined(SVN_DEBUG_ERROR)
- apr_pool_cleanup_kill (oldpool, NULL, err_abort);
+ apr_pool_cleanup_register (pool, chain, err_abort, NULL);
 #endif
+
+ /* Destroy the new error chain. */
   apr_pool_destroy (oldpool);
 }
 
@@ -186,7 +199,9 @@
   if (err)
     {
 #if defined(SVN_DEBUG_ERROR)
- apr_pool_cleanup_kill (err->pool, NULL, err_abort);
+ while (err->child)
+ err = err->child;
+ apr_pool_cleanup_kill (err->pool, err, err_abort);
 #endif
       apr_pool_destroy (err->pool);
     }

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org