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

[Jeffrey H. <jef...@aj...>]

Make sure that you always also run the full test suite over the code
when changing these things.  Even when it seems unrelated, just make
sure to run the full test suite before any check-ins.  I made what I
thought was a refcount fix to the button code like Scott mentions
below, which turned out to be incorrect.  This didn't turn up in
button.test (which is the only one I ran stupidly enough).  It turned
up in canvImg.test.

I didn't take the time to research why what I did was wrong, but
obviously it was more correct than the crash I got afterwards...

Jeffrey Hobbs                     Tcl Ambassador
ho...@Aj...          Ajuba Solutions (nee Scriptics)

> -----Original Message-----
> From: Scott Stanton [mailto:st...@aj...]
> Sent: Monday, August 28, 2000 10:25 AM
> To: Laurent Duperval
> Cc: tc...@aj...; jc...@gm...; a.k...@we...;
> pe...@ii...; j.n...@ch...; bi...@ny...
> Subject: [TCLCORE] Re: -underline patch v. 0.2 [tcl gateway]
> 
> 
> 
> 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.
> 

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