You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2008/04/29 15:30:41 UTC

Re: svn commit: r30835 - in trunk/subversion: include libsvn_subr

On Tue, Apr 29, 2008 at 03:42:34AM -0700, danielsh@tigris.org wrote:
> Author: danielsh
> Date: Tue Apr 29 03:42:34 2008
> New Revision: 30835
> 
> Log:
> Do not abort in svn_handle_error2().
> 
> Suggested by: stsp

It wasn't just me! brane should be added to the list of suggesters:

>    fflush(stream);
>    if (fatal)
> -    /* XXX Shouldn't we exit(1) here instead, so that atexit handlers
> -       get called?  --xbc */
> -    abort();


> +    {
> +      /* Avoid abort()s in maintainer mode. */
> +      svn_error_clear(err);
> +
> +      /* We exit(1) here instead of abort()ing so that atexit handlers
> +         get called. */
> +      exit(EXIT_FAILURE);

I'd prefer:

  /* We exit() here instead of abort()ing so that atexit handlers

Saying "exit(1)" and doing "exit(EXIT_FAILURE)" is apparently the
same thing, but it looks odd. </bikeshed>

> +    }
>  }

-- 
Stefan Sperling <st...@elego.de>                    Software Monkey
 
German law requires the following banner :(
elego Software Solutions GmbH                            HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 
13355 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                               CEO: Olaf Wagner
 
Store password unencrypted (yes/no)? No

Re: svn commit: r30835 - in trunk/subversion: include libsvn_subr

Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Stefan Sperling wrote on Tue, 29 Apr 2008 at 17:30 +0200:
> On Tue, Apr 29, 2008 at 03:42:34AM -0700, danielsh@tigris.org wrote:
> > Author: danielsh
> > Date: Tue Apr 29 03:42:34 2008
> > New Revision: 30835
> > 
> > Log:
> > Do not abort in svn_handle_error2().
> > 
> > Suggested by: stsp
> 
> It wasn't just me! brane should be added to the list of suggesters:
> 

Propedited.  Sorry, brane.

Further credit to Malcolm, whose r24415 log message was very helpful in 
analyzing the aborts (maintainer-mode and otherwise).  (But he didn't 
suggest this specific change)

> >    fflush(stream);
> >    if (fatal)
> > -    /* XXX Shouldn't we exit(1) here instead, so that atexit handlers
> > -       get called?  --xbc */
> > -    abort();
> 
> 
> > +    {
> > +      /* Avoid abort()s in maintainer mode. */
> > +      svn_error_clear(err);
> > +
> > +      /* We exit(1) here instead of abort()ing so that atexit handlers
> > +         get called. */
> > +      exit(EXIT_FAILURE);
> 
> I'd prefer:
> 
>   /* We exit() here instead of abort()ing so that atexit handlers
> 
> Saying "exit(1)" and doing "exit(EXIT_FAILURE)" is apparently the
> same thing, but it looks odd. </bikeshed>
> 

When I wrote "exit(1)" I meant "exit() with a non-zero code".

+1 if you want to change it.

> > +    }
> >  }
> 
> 

Thanks Stefan.

Daniel

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