[m-rev.] Fwd: for review: fix obsolete tcl-tk code in extras/graphics/mercury_tcltk

[Julien Fischer]

Hi Fabrice,

On Wed, 5 Jan 2022, Fabrice Nicol wrote:

> Yes, Julien, you are (theoretically) right. You will find attached a
> new patch that takes your review into account.
>
> Practically yet, for reasons I still poorly understand, using the setter 
> functions (Tcl_SetResult or Tcl_SetObjResult) as you indicate in your review 
> causes a runtime crash on using the 'calc' executable built with the tcltk 
> library code fixed along these lines, __unless the TCL_STATIC memory 
> management flag__ is set in the third argument of the setters.
>
> There may be some C-memory management issue, as the TCL-TK setter functions 
> perform C-style memory allocation under the hood. I guess that the dynamic 
> version (TCL_DYNAMIC, which is often the preferred option) is somehow 
> incompatible with the Mercury GC, and I am not quite sure that using 
> TCL_STATIC releases all tcl-tk-allocated memory on exit.

According the Tcl/Tk documentation TCL_DYANMIC is for when the memory
referenced by the result field is allocated using the Tcl_Alloc()
function.  (Memory allocated by Mercury's GC is definitely *not*
allocated using this function.)

I think the safest approach here is to get the result string from
Mercury, copy it into memory allocated using Tcl_Alloc() and then
pass that to Tcl_SetResult().

> Oddly enough, 'calc' works just fine (for all arithmetic operations) if the 
> getter function Tcl_GetStringResult is used instead of the setters, as in my 
> earlier proposed patch. This may be caused by the fact that the underlying 
> 'interp' structures are not strictly write-protected (this is not C++ code), 
> which may explain why the getter function is optimized away by the compiler 
> as an alias for the 'result' field of 'interp', as in prior code. This may be 
> a case of efficient, yet unclean coding.

The explanation is simpler than that: nothing in mtk.m or calc.m is
actually looking at the result field of the interpreter.  It's only set
in two places, in mtk.m, and both those places set it to the empty
string.

Julien.