Tcl Source Code

View Ticket
Login
Ticket UUID: 2997642
Title: many type casts needed when using Tcl_Pkg* API
Type: Patch Version: None
Submitter: nijtmans Created on: 2010-05-06 14:37:11
Subsystem: 39. Package Manager Assigned To: nijtmans
Priority: 9 Immediate Severity:
Status: Closed Last Modified: 2010-09-09 04:14:37
Resolution: Fixed Closed By: nijtmans
    Closed on: 2010-09-08 21:14:37
Description:
Since Tcl 8.6, all stub pointers became "const". The
Tcl_Pkg* functions have a ClientData parameter for
registering them. Because this parameter is not const,
it means that in tclBasic.c, tclTomMathInterface.c,
tclStubLib.c, and tclTomMathStubLib.c type casts
need to be used in order to silence the compiler
warning. There must be a better way.

Therefore, I suggest to change the API of the
following 4 functions:
    Tcl_PkgProvideEx
    Tcl_PkgRequireEx
    Tcl_PkgPresentEx
    Tcl_PkgRequireProc
so that they can be used without
type casts in this situation.

Attached patch shows how to do that. This
is 100% source and binary compatible. The
real changes are only in tcl.decls (and the
generate tclDecls.h) and tclPkg.c. The
other files don't need to be modified, but
are just a demonstration how they
can be cleaned up without type
casts after the signature change.

doc update is in the patch as well.
User Comments: nijtmans added on 2010-09-09 04:14:37:

allow_comments - 1

Done, so can be closed

nijtmans added on 2010-09-01 04:02:35:
Remaining part committed. tclStubLib.c and tclTomMathStubLib.c
change reverted as Twylite reported to me that MSVC++ 6.0 gives
a warning with this construct. :-(

jenglish added on 2010-08-31 01:35:03:
Remembering that "ClientData" == "void *":

Previous signature had:
   Tcl_PkgProvideEx(...., void *clientData);
   Tcl_PkgRequireEx(..., void **clientData_rtn);

IOW: put pointer-to-T in, get void * back out,  which the caller must convert back to a pointer-to-T.

(Note that the PkgRequire() call did not require an explicit cast even under the old signature  when using the usual idiom, and the PkgProvide call only needed a cast-away-from-const cast if the clientData is a pointer-to-const-T.)

New signature has:
   Tcl_PkgProvideEx(...., const void *clientData);
   Tcl_PkgRequireEx(..., void *rtn);

IOW: put a pointer-to-T or pointer-to-const-T in, get *anything* back out.  This pokes an even bigger hole in the type system (including internally removing 'const' qualifiers), but since 'void *' is already involved that's probably not a big deal; we're already relying on the caller to get things right, the compiler won't help here.

As long as it does not cause a regression of #1091431, it's probably OK.  (Basically: make sure that if you're changing the usual idiom 

   void *pkgData = NULL;
   if (Tcl_PkgRequireEx(...., &pkgData) != NULL && pkgData != NULL)
       fooStubsPtr = pkgData;

to 

   Tcl_PkgRequireEx(..., &fooStubsPtr);

make sure that 'fooStubsPtr' doesn't get stomped in cases where the PkgRequire() fails.

(It doesn't look like that should happen, but I can't really tell; the PkgRequire() codepath has gotten a lot more convoluted since last I looked at it.)

nijtmans added on 2010-08-30 20:49:34:
Partly committed (except for Tcl_PkgProvideEx
signature change).

Joe, please evaluate. I am convinced this is a
pure improvement, and 100% binary and source
compatible, but if you can find any
problem with it feel free to revert.

The tclStubLib.c and tclTomMathStubLib.c
modifications serve as demonstration how
this change can benefit code.

nijtmans added on 2010-05-06 21:38:15:

File Added - 373071: tcl-pkg.patch

Attachments: