Re: [RFC] Patch for gnats pr 2495

[Phil Muldoon <pmuldoon at redhat dot com>]

Tom Tromey wrote:
> > > > > > "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> > > > > >             
>
> Phil> This patch addresses that by gating access to std::terminate in an
> Phil> inferior function call.
>
> Sounds good.  The patch looks nice!  In fact the majority of my
> comments are on the string constants :-)
>   

Thanks for taking a look at it.

> Phil> +int unwind_on_terminating_exception_p = 1;
>
> I think this should be static.
>   


Ok.


> Phil> +  fprintf_filtered (file, _("\
> Phil> +Unwinding of stack from a std::terminate call originating from \
> Phil> +default C++ exception handler is %s.\n"),
>
> This is a nit -- for a one-line string constant that is too long, I
> would just have it overflow the 80 column boundary rather than using
> "\" to wrap.  Or, you could use string concatenation.  Or -- perhaps
> best -- try to reword it to fit.
>   

Ok will reword it.

> Phil> +  /* Only clean up terminating exception breakpoint if it was set */
> Phil> +  if (terminate_bp != NULL)
> Phil> +    make_cleanup_delete_breakpoint (terminate_bp);
>
> Why not create the cleanup when the breakpoint is made?
> If there's a reason, I'd suggest documenting it here.
>   


Will look into this further.


> Phil> +		  error (_("\
> Phil> +The program being debugged entered a std::terminate call which would\n\
> Phil> +have terminated the program being debugged.  GDB has restored the\n\
> Phil> +context to what it was before the call.\n\
> Phil> +To change this behaviour use \"set unwind-on-terminating-exception \
> Phil> +off\"\n\
>
> The line-wrap after "exception" is odd.  I had to read it twice... it
> would be better not to have to wrap here.  If this doesn't fit, I
> suggest a newline after "use".
>   


Ok will fix (word-wrap mess-up).


> Phil> +The unwind-on-terminating-exception lets the user determine what\n\
>
> "The unwind-on-terminating-exception" reads strangely to me.
>   


Yeah that's the name of the set/show flag. So I kept it there. Not 
particularly very happy with the name. Suggestions welcome ;)


> Phil> +C++ exceptions are often handled out-of-frame, but the constraints\n\
> Phil> +of the call-dummy can fool the unwinder into thinking there is no\n\
> Phil> +exception handler, and calls the default handler. This in turns\n\
> Phil> +calls std::terminate, which will terminate the inferior.\n\
>
> I think this bit could be omitted without loss of clarity.
>
> Perhaps this would be better in the manual?
> (BTW -- a user-visible change like this should have a gdb.texinfo
> patch too...)
>   


Ok, did not add anything to the gdb.texinfo, good catch. Will reword and 
rewrite the help, add a documentation patch to gdb.texinfo.


> Phil> [ .. tests .. ]
>
> Nice.
>   


One thing I forgot to do was clean-up my resulting test executable with 
a patch to Makefile.in. Will fix.

Regards

Phil