Tcl Source Code

View Ticket
Login
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"?

Attachments: