You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Malcolm Rowe <ma...@farside.org.uk> on 2007/01/07 14:58:25 UTC

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

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

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

Posted by Peter Lundblad <pl...@google.com>.
Malcolm Rowe writes:
> 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.
> 
+1.

Thanks,
//Peter

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

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

Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Mon, Jan 08, 2007 at 10:25:07AM -0800, Daniel Rall wrote:
> On Sun, 07 Jan 2007, Malcolm Rowe wrote:
> > Enable error lifetime checking by default in maintainer mode, causing
> ...
> 
> I'm in favor of this change.

r22934, thanks, both.

> Alternately, SVN_DEBUG_ERROR could be
> defined when SVN_DEBUG is defined.  This is only slightly more
> complex, but does leave in a convenient toggle for people who need
> more control over things.
> 

I thought about doing something like that (actually, defining  an
SVN_DEBUG_NO_ERROR to disable it instead, inverting the #ifdefs), but
I thought it would be easier to see if anyone violently objected to
enabling it by default instead.

I figure if anyone really wants to disable it, 'svn merge -c-22934 .' is
just as easy as commenting out an #ifdef SVN_DEBUG / #define SVN_DEBUG_ERROR.

Regards,
Malcolm

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

Posted by Daniel Rall <dl...@collab.net>.
On Sun, 07 Jan 2007, Malcolm Rowe wrote:
...
> [[[
> 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)
...

I'm in favor of this change.  Alternately, SVN_DEBUG_ERROR could be
defined when SVN_DEBUG is defined.  This is only slightly more
complex, but does leave in a convenient toggle for people who need
more control over things.

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