#1025 Possible problem with stubs

closed-accepted
tcl (60)
5
2009-07-03
2009-06-22
No

The initialization code for Tcl stubs currently looks like the following:

#ifdef USE_TCL_STUBS
if (Tcl_InitStubs(interp, (char*)"8.1", 0) == NULL) {
return TCL_ERROR;
}
#endif

The hard-coded versionstring "8.1" should be replaced by the macro TCL_VERSION - this requests the stubs interface for the version against which the extension is compiled. Also, to simplify Tk extensions, the init code could include a corresponding Tk_InitStubs call. (this got me crazy when I tried to port a working Tk extension from Linux to MacOSX, which was used inside a starkit.)
See the attached patch

Discussion

  • Christian Gollwitzer

    Patch for the stubs problem

     
  • Olly Betts

    Olly Betts - 2009-06-23

    Do you have a pointer to Tcl documentation that says this is how it should be done? Or failing that an explanation of why the current code breaks for you?

    I'm not a Tcl expert, but my understanding was that using stubs allowed the built bindings to work with any Tcl >= 8.1, and this change seems like it would break that by requiring a Tcl version >= the one actually built against.

    I've just done a quick test and I can use SWIG wrappers built against Tcl8.4 with Tcl8.3 or Tcl8.5 (those being the versions which are available as packages for Ubuntu jaunty).

    As for Tk_InitStubs(), that really only wants to get called if you are using Tk, which not everyone will be...

     
  • Olly Betts

    Olly Betts - 2009-06-23

    Ah, I see the patch conditionalises on USE_TK_STUBS, so that part seems fine to me.

     
  • Christian Gollwitzer

    I've stumbled across this newsgroup thread:
    http://groups.google.de/group/comp.lang.tcl/msg/6975f45c95c063f5?hl=de
    which alerted me since I had to hack around in the SWIG generated code. If I understand the stubs mechanism correctly, the init_stubs functions populate at runtime a table of function pointers suitable for the requested ABI version. At least in Tk, there are some incompatibilites between 8.4 and 8.5 - in my case it was Tk_PhotoSetSize() which has a different signature in 8.5. The version compiled against 8.5, but loaded into 8.4 by "lying" to Tk_InitStubs() crashes 8.4. If compiled against 8.4 headers, it works in both 8.4 and 8.5 as expected.

    The short story: As long as you uses only the SWIG generated code and don't call Tcl_* functions by yourself, it should work because the functions that SWIG uses have never ever changed since 8.1. However if the SWIG user calls a function from 8.5 in his C-code, links to 8.5 stubs and loads the extension into 8.4, it should crash.

    I must admit that in Tcl-only extensions this might be a theoretical problem. Maybe there is a way to allow the advanced user to specify the version? Something like

    #define TCL_STUBS_COMPATIBILITY_VERSION "8.1"
    Tcl_InitStubs(interp, (char*)TCL_STUBS_COMPATIBILITY_VERSION, 0)

    which can be redefined? BTW: What's the reason for the (char*) cast?

     
  • Olly Betts

    Olly Betts - 2009-06-23

    Thanks for the link to the thread - that gives some useful background. However, other messages in the thread seem to support my understanding of how the stubs should work:

    http://groups.google.com/group/comp.lang.tcl/msg/170a1f5596cc2429

    http://groups.google.com/group/comp.lang.tcl/msg/da258c015c641596

    It sounds to me like either Tk doesn't make the same stubs guarantees which Tcl does, or someone accidentally broke these for some functions between 8.4 and 8.5. Of course such an error may have happened or could happen for Tcl too...

    I don't think we have an active Tcl backend maintainer for SWIG currently, since this didn't get automatically assigned to anyone, so I guess I should take care of this since I do at least use the Tcl backend.

    So, what to do?

    I think the current behaviour is probably the best overall default for Tcl, but allowing overriding the literal "8.1" with another literal value or TCL_VERSION seems a good idea.

    It seems wise for the documentation to cover this and probably advise people distributing binary versions that it is safest to build against the oldest version they want to support (as the message you linked to said), especially if they call Tcl or Tk API functions other than those which the SWIG generated code already does.

    It would definitely be useful to allow initialising the Tk stubs too. Not sure what's the best default version there, but it needs to be the same as for Tcl for sanity, so I guess the question is whether to change the default if Tk stubs are enabled. I tend to think it is better not to as it depends what Tk functions you are calling...

    I think the (char*) cast is probably for some older Tcl version which didn't have a "const" in the prototype for Tcl_InitStubs(). Older C compilers are pretty forgiving of this with string literals, but newer ones aren't, and C++ doesn't allow it.

     
  • Olly Betts

    Olly Betts - 2009-06-23
    • assigned_to: nobody --> olly
     
  • Christian Gollwitzer

    New patch with macro SWIG_TCLSTUB_VERSION

     
  • Christian Gollwitzer

    I've been digging into the stubs sources of Tk, and it seems, that the API change was a deliberate design decision. They renamed Tk_PhotoSetSize() to Tk_PhotoSetSize_Panic() and introduced the new Tk_PhotoSetSize() function.

    I agree that the current behaviour is the best overall default. How about the new patch? It defines the macro SWIG_TCLSTUB_VERSION to be "8.1", which can be overriden by the user similar to SWIG_version.

     
  • Olly Betts

    Olly Betts - 2009-06-30

    Seems reasonable, with the SWIG_TCLSTUB_VERSION define wrapped in "#ifndef SWIG_TCLSTUB_VERSION" to allow the user to override it (e.g. with -DSWIG_TCLSTUB_VERSION=TCL_VERSION or similar).

    Also I think SWIG_TCL_STUBS_VERSION would be a more consistent name (since it's USE_TCL_STUBS not USE_TCLSTUB...)

    Don't worry about updating the patch though.

    I'll wait a few days to give others a chance to comment and apply this if there's no dissent.

     
  • Olly Betts

    Olly Betts - 2009-07-03

    Fixed in SVN trunk r11353, so should be in 1.3.40 when it is released.

     
  • Olly Betts

    Olly Betts - 2009-07-03
    • status: open --> closed-accepted
     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks