Ticket UUID: | 533907 | |||
Title: | Tcl_EvalObjv: huh?! | |||
Type: | Bug | Version: | obsolete: 8.4a5 | |
Submitter: | dgp | Created on: | 2002-03-23 05:16:19 | |
Subsystem: | 45. Parsing and Eval | Assigned To: | msofer | |
Priority: | 7 High | Severity: | ||
Status: | Closed | Last Modified: | 2002-03-25 02:05:51 | |
Resolution: | Fixed | Closed By: | msofer | |
Closed on: | 2002-03-24 19:05:51 | |||
Description: |
The routine Tcl_EvalObjv is a monstrosity of obfuscation. Surely we can rewrite it to be more clear. If I'm deciphering it correctly, it appears that when evaluation returns TCL_ERROR, then Tcl_LogCommandInfo will be called -- *EXCEPT* when it has been detected that a trace on the command might be in place. In that case, cmdLen will not be zero, and the "goto cmdtraced" will not be taken. That's good, because we don't need to generate the cmdString since it's already done. But it's bad because without that goto, we fall out of the switch and never call Tcl_LogCommandInfo, which is what we constructed the cmdString for in the first place! Recent efforts to improve command tracing (TIPs 32, 79) and pending efforts to expose command tracing at the script level (TIP 62) will make it more important to get this case correctly handled. | |||
User Comments: |
msofer added on 2002-03-25 02:05:51:
Logged In: YES user_id=148712 Patch committed. dgp added on 2002-03-24 23:38:22: File Added - 19943: 533907.patch2 dgp added on 2002-03-24 23:38:21: Logged In: YES user_id=80530 I think the duplication is fine; it's small enough. If anyone does want to get rid of the duplication, then I say write a new function instead of goto. Here's a revised patch that changes only comments and formatting to keep things within 80 columns. msofer added on 2002-03-24 21:59:30: File Added - 19936: 533907.patch Logged In: YES user_id=148712 Indeed, it is quite obscure coding - and wrong too, as you point out. I'd propose to duplicate the small string-generation code for the sake of clarity, as implemented in the enclosed patch. Comments? Should we avoid the code duplication with a couple of "goto"? |