[TCLCORE] Re: -underline patch v. 0.2 [tcl gateway]

[Scott S. <st...@aj...>]

Laurent Duperval said:

 > +     /*
 > +      * Incr valuePtr before Decr, in case they point to the same 
object.
 > +      * We could also do some short-circuiting in that case, but it
 > +      * shouldn't happen in practice.
 > +      */
 > +     Tcl_IncrRefCount(valuePtr);
 > +     butPtr->textPtr = valuePtr;
 > +     /* Tcl_IncrRefCount(butPtr->textPtr); */
 > + 	ButtonComputeUnderline(butPtr);
 >       Tcl_DecrRefCount(butPtr->textPtr);
 >       butPtr->textPtr = valuePtr;
 >       Tcl_IncrRefCount(butPtr->textPtr);

This code seems to be confused.  You should replace the code above with:

Tcl_IncrRefCount(valuePtr);
Tcl_DecrRefCount(butPtr->textPtr);
butPtr->textPtr = valuePtr;
ButtonComputeUnderline(butPtr);



 > + 	/* 
 > + 	 * At this point, we have a correct value for -text. Copy that value 
in
 > to
 > + 	 * the display pointer. Should the previous value be discarded? 
Hmmmm..
 > .
 > + 	 */
 > + 	butPtr->textDisplayPtr = Tcl_DuplicateObj(butPtr->textPtr);

Again, the reference counting is off here.  You should say:

    Tcl_Obj *newTextPtr;

    newTextPtr = Tcl_DuplicateObj(butPtr->textPtr);
    Tcl_IncrRefCount(newTextPtr);
    if (butPtr->textDisplayPtr) {
	Tcl_DecrRefCount(butPtr->textDisplayPtr);
    }
    butPtr->textDisplayPtr = newTextPtr;

This will ensure that the old display string (if one exists) is released.  
Also, you need to add code to ButtonCreate to initialize the butPtr->
textDisplayPtr to NULL when the button structure is first created.  You 
should also verify that there's no way to get to the display code with a 
null textDisplayPtr.

--Scott


--
The TclCore mailing list is sponsored by Ajuba Solutions
To unsubscribe:  email tcl...@aj... with the 
                 word UNSUBSCRIBE as the subject.