This is the mail archive of the insight@sources.redhat.com mailing list for the Insight project.


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

Re: Error message revelation


>>>>> "Keith" == Keith Seitz <keiths@cygnus.com> writes:

Keith>    Tcl_WrongNumArgs (...);
Keith>    return TCL_ERROR;

Yeah, this is what I wanted to be able to do.

Keith> 2000-11-29  Tom Tromey  <tromey@cygnus.com>
Keith>         * gdbtk-cmds.c (call_wrapper): Don't reset result if wrapped
Keith>         command returned error.

I did this because I didn't like the old rules, and nobody objected to
the change -- I'm sure the discussion (perhaps brief) is still in the
archives.

As I recall, what I found confusing was this:

When I'm writing a new Tcl command in gdbtk-cmds.c, it seems logical
to me that it should be like writing any other random Tcl command.
The point of call_wrapper should be, in my opinion, that the inner
oddities of the gdb exception handling mechanism are kept from the Tcl
code.

Also, requiring a new flag to be set on every error return seemed
redundant.  I couldn't think of a situation where you would want to
return a TCL_ERROR but have the result text taken from some other
source.

Keith> However, I think that the latter, more verbose code is much
Keith> easier to remember... The result is ALWAYS taken from
Keith> result_ptr->obj_ptr EXCEPT if GDBTK_IN_TCL_RESULT is
Keith> specified. That's much simpler than: result is taken from
Keith> result_ptr->obj_ptr EXCEPT if there was an error OR
Keith> GDBTK_IN_TCL_RESULT is set.

As I recall, when looking around the code in gdbtk, there were
numerous returns where the writer forgot to set GDBTK_IN_TCL_RESULT.

Personally I think this approach is a bug factory.  They aren't
critical bugs, perhaps, but I'm guessing people will often forget to
set the new flag.

I agree that the change I made isn't ideal.  As I recall it was just a
side track off some other problem I was working on.  I think the
call_wrapper error handling code is ripe for a deeper rethink than
just toggling this patch back out.  IMHO.

OTOH, a cheap-and-dirty method would be use have a new
`RETURN_TCL_ERROR' macro that sets the flag.  Then you could find all
the problem spots by a simple `grep "return TCL_ERROR"', both now and
in the future.

Tom


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