Re: [RFC] Patch for gnats pr 2495

[Tom Tromey <tromey at redhat dot com>]

>>>>> "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 :-)

Phil> +int unwind_on_terminating_exception_p = 1;

I think this should be static.

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.

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.

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".

Phil> +The unwind-on-terminating-exception lets the user determine what\n\

"The unwind-on-terminating-exception" reads strangely to me.

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...)

Phil> [ .. tests .. ]

Nice.

Tom