This is the mail archive of the mailing list for the Archer project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] Patch for gnats pr 2495

Tom Tromey wrote:
"Phil" == Phil Muldoon <> 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 \
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 .. ]


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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]