This is the mail archive of the
mailing list for the Archer project.
Re: [RFC] Patch for gnats pr 2495
- From: Phil Muldoon <pmuldoon at redhat dot com>
- To: Tom Tromey <tromey at redhat dot com>
- Cc: Project Archer <archer at sourceware dot org>
- Date: Mon, 20 Oct 2008 10:15:50 +0100
- Subject: Re: [RFC] Patch for gnats pr 2495
- References: <48F344BB.firstname.lastname@example.org> <email@example.com>
Tom Tromey wrote:
"Phil" == Phil Muldoon <firstname.lastname@example.org> 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.
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 \
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
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 .. ]
One thing I forgot to do was clean-up my resulting test executable with
a patch to Makefile.in. Will fix.