From: Donald G P. <dg...@ni...> - 2008-02-07 17:41:59
|
dg...@ni... <dg...@ni...> wrote: >> However it doesn't have any risk of leaving behind >> an invalid pointer to the former internals of a >> tclListType Tcl_Obj that got lost in the shimmer. >> It's those invalid pointers that present a crash >> risk that got hunted. Alexandre Ferrieux wrote: > Hum, unless I'm mistaken, it seems that the 1.98 tclCmdIL.c (not > checked its relationship with Tcl releases) doesn't have this crash > risk either. ... To be more explicit about it, on lines 2992-3017, the variable elemPtrs can hold an invalid pointer value. Any revisions to the code that attempt to use that variable within those lines will create a crashing bug. That's the crash risk. As you've observed, the 8.4.18 implementation of [lrange] dodges this risk by correctly re-calling T_LOGE to re-establish the validity of elemPtrs before using it. To a first reader of the code, though, this is bizarre. "Why are we calling that again? We already called it." The 8.4.18 implementation of [lrange] also addresses this question by including comments explaining why the second T_LOGE call is needed. So, yes [lrange] is bug free in 8.4.18. But it's also implemented with what I consider to be "tricky code". Its flow isn't the simple sequence of steps that come to mind from the spec. for [lrange]. Comments are needed to explain unexpected things, and failure to abide by those comments will lead to crashes. In Tcl 8.5 development the "list" Tcl_ObjType got a new intrep, which makes cheap copying possible. When cheap copies are available, a cheap alternative implementation for [lrange] is possible that doesn't require any tricky code. That's what I did while on the hunt for crash bugs among T_LOGE callers. You observe correctly that "cheap" != "zero cost". I think I agree that the best answer is that the first T_LOGE call shouldn't be there in the first place. Delay the T_LOGE call until we're actually going to use elemPtrs, and use T_LOL as the first call used, to verify listiness and get the length. Having said that, I'm unlikely to revise the 8.4 implementation simply because we're so close to end of life there that improvements that only make the code more maintainable have little value since there's no more maintaining to be done. > (3) I have also imagined an alternative way. The problem in all the > above is indeed the fact that index-decoding needs to know the list > length in order to process indices like "end-N". Not so. The T_LOL or T_LOGE calls do tell us the length, which we do use, but the other function they perform is to test whether the argument is a valid list. We have to do that anyway (and we have to do it at that point to preserve the error throwing properties of [lrange]) so we might as well get the length, and use the existing support routines. -- | Don Porter Mathematical and Computational Sciences Division | | don...@ni... Information Technology Laboratory | | http://math.nist.gov/~DPorter/ NIST | |______________________________________________________________________| |