Fixed bug in the patch where Tcl_NamespaceDelete was
decrementing the reference count on the unknownHandlerPtr of
the namespace but not then setting it to NULL. This resulted
in a later double free and possible segmentation fault under
certain conditions, such as those produced by
autoMkindex.test. Added a new test for this condition.
Thanks to Donald Porter for pointing out problem.
tip181-6.patch is the latest.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
In the routine TclEvalObjvInternal,
the Tcl_Obj's newObjv[1] through
newObjv[handlerObj-1] do not have
their refcounts bumped before the
recursive call to TEOI. Looks unsafe
to me.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
while merging with dgp-refactor
branch, the question arose whether
this patch is properly handling
the TCL_EVAL_INVOKE and/or
TCL_EVAL_GLOBAL cases. Needs a
careful check, and possibly new
tests.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
When the elements of the
unknownHandlerPtr are retrieved
within TclEvalObjvInternal, there's
a check for valid list, and return
of TCL_ERROR if not.
This should not be possible. Valid
list checks are done during storage
of the handler value. I think it
would be better either to not check
for the error condition, or to check
and react to it with a Tcl_Panic.
As is, I think the error message
and ::errorInfo would not be helpful
at all.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I wasn't sure about bumping the refcounts of the other
elements of the unknownHandler. The safest thing to do would
be to put the increments in.
Regarding TCL_EVAL_DIRECT/GLOBAL, I'm not sure what is the
correct behaviour with these flags. The current code was a
best guess based on what was already there. Certainly, it
could stand some reviewing.
You are correct that the unknown handler cannot possibly be
anything other than a valid list (or NULL). The check can be
changed to a panic, or removed entirely.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
The issues with execution traces
properly dealing with the
TCL_EVAL_* flag values are much
more difficult to address, and
they're really orthogonal to
the new [namespace unknown]
command.
Revised patch is limited to
fixing [namespace unknown] bugs.
A separate bug report will be
filed for the trace issues.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
First stab reference implementation of TIP 181
New, improved, patch with added correctness.
Handfull of improvements and some changes to the TIP
Updated patch with public C-API
Logged In: YES
user_id=102050
Latest patch tip181-4.patch will hopefully be the last -- now includes
documentation and C-API (+stubs etc). Passes test-suite.
Updated to current HEAD
Logged In: YES
user_id=102050
Updated patch to latest CVS HEAD -- tip181-5.patch is now
the current version.
Fixed bug in Tcl_NamespaceDelete handling
Logged In: YES
user_id=102050
Fixed bug in the patch where Tcl_NamespaceDelete was
decrementing the reference count on the unknownHandlerPtr of
the namespace but not then setting it to NULL. This resulted
in a later double free and possible segmentation fault under
certain conditions, such as those produced by
autoMkindex.test. Added a new test for this condition.
Thanks to Donald Porter for pointing out problem.
tip181-6.patch is the latest.
Logged In: YES
user_id=80530
Updated patch to HEAD.
Logged In: YES
user_id=80530
958222.patch committed for 8.5a4
Logged In: YES
user_id=80530
In the routine TclEvalObjvInternal,
the Tcl_Obj's newObjv[1] through
newObjv[handlerObj-1] do not have
their refcounts bumped before the
recursive call to TEOI. Looks unsafe
to me.
Logged In: YES
user_id=80530
while merging with dgp-refactor
branch, the question arose whether
this patch is properly handling
the TCL_EVAL_INVOKE and/or
TCL_EVAL_GLOBAL cases. Needs a
careful check, and possibly new
tests.
Logged In: YES
user_id=80530
When the elements of the
unknownHandlerPtr are retrieved
within TclEvalObjvInternal, there's
a check for valid list, and return
of TCL_ERROR if not.
This should not be possible. Valid
list checks are done during storage
of the handler value. I think it
would be better either to not check
for the error condition, or to check
and react to it with a Tcl_Panic.
As is, I think the error message
and ::errorInfo would not be helpful
at all.
Logged In: YES
user_id=102050
I wasn't sure about bumping the refcounts of the other
elements of the unknownHandler. The safest thing to do would
be to put the increments in.
Regarding TCL_EVAL_DIRECT/GLOBAL, I'm not sure what is the
correct behaviour with these flags. The current code was a
best guess based on what was already there. Certainly, it
could stand some reviewing.
You are correct that the unknown handler cannot possibly be
anything other than a valid list (or NULL). The check can be
changed to a panic, or removed entirely.
Logged In: YES
user_id=80530
Attaching supplemental patch
addressing these issues.
I won't commit this until I
construct tests for the test
suite that demo its effects.
Logged In: YES
user_id=80530
Updated patch includes more
[namespace unknown] tests
that illustrate the bugs fixed
by the supplemental patch.
please review.
still needs trace tests.
Logged In: YES
user_id=80530
The issues with execution traces
properly dealing with the
TCL_EVAL_* flag values are much
more difficult to address, and
they're really orthogonal to
the new [namespace unknown]
command.
Revised patch is limited to
fixing [namespace unknown] bugs.
A separate bug report will be
filed for the trace issues.