From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Tromey To: Syd Polk Cc: Insight List Subject: Re: Patch: fixlet in gdbtk-cmds.c Date: Tue, 28 Nov 2000 17:32:00 -0000 Message-id: <87wvdn4krn.fsf@creche.cygnus.com> References: <4.2.0.58.20001128125809.01a7f100@pop.cygnus.com> X-SW-Source: 2000-q4/msg00315.html Syd> Approved, although I would have preferred using Syd> Tcl_WrongNumArgs. That would give a slightly different error Syd> message, however, and that might break some tests. I looked at this some more. No gdbtk tests test any command for the wrong number of arguments. So we're free to change this however we want. Some of the code in gdbtk-cmds.c is quite bad. For instance there are functions which do no checking, or incorrect checking, of the number of arguments they are given. I discovered that if I just use Tcl_WrongNumArgs and return TCL_ERROR, I don't get the right result, because you also have to do this: result_ptr->flags |= GDBTK_IN_TCL_RESULT; Note that I changed all the functions in gdbtk-cmds.c that did any argument number checking to use Tcl_WrongNumArgs. However this issue has to be resolved first :-(. I didn't bother fixing all the commands, since my time on Insight is limited and I doubt this sort of cleanup is very high priority. If we really do have to set result_ptr->flags, that's fine -- but I'd like to add a macro like this: #define RETURN_TCL_ERROR \ result_ptr->flags |= GDBTK_IN_TCL_RESULT; \ return TCL_ERROR Then we can just use `RETURN_TCL_ERROR;' all over. This is ugly, but imho more maintainable than remembering to put an assignment everywhere we return TCL_ERROR. OTOH I don't understand this comment in call_wrapper: /* * Now copy the result over to the true Tcl result. If GDBTK_TO_RESULT flag * bit is set , this just copies a null object over to the Tcl result, * which is fine because we should reset the result in this case anyway. */ First, the comment seems to lie. It mentions GDBTK_TO_RESULT, which makes sense, but the code actually checks for GDBTK_IN_TCL_RESULT. Ok, I just read the comments in gdbtk.h and it makes a bit more sense. But wouldn't it be easier to adopt the heuristic that if a wrapped command returns TCL_ERROR then we should assume that the interpreter's result is already correctly set? Are there cases where we explicitly (as opposed to via error()) return TCL_ERROR but rely on the call wrapper to set the result? That seems strange. Tom