Ticket UUID: | 2994165 | |||
Title: | Signature of Tcl_FSGetNativePath and TclpDeleteFile | |||
Type: | Patch | Version: | None | |
Submitter: | nijtmans | Created on: | 2010-04-29 13:18:56 | |
Subsystem: | 37. File System | Assigned To: | nijtmans | |
Priority: | 5 Medium | Severity: | ||
Status: | Closed | Last Modified: | 2010-08-25 08:08:34 | |
Resolution: | Fixed | Closed By: | nijtmans | |
Closed on: | 2010-08-25 01:08:34 | |||
Description: |
Most file system API functions which have a native path as parameter, use a "void *" for that. But there are two exceptions, which result in the need of extra (unneccessary) type casts. This patch changes the parameter type from "char *" to "void *", and adds the necessary machinery to keep source and binary compatibility. Currently, the documentation for Tcl_FSGetNativePath mentions: This function is a convenience -wrapper around \fBTcl_FSGetInternalRep\fR, and assumes the native-representation is string-based The effect of this patch is that the mentioned assumption is relaxed. The patch contains the doc update as well. Tested on Linux and win32 (MSVC 6.0/mingw/CYGWIN) Adreas, Kevin, any remarks? | |||
User Comments: |
nijtmans added on 2010-08-25 08:08:34:
allow_comments - 1 Fixed in HEAD. Should not be backported to Tcl 8.5 or lower! nijtmans added on 2010-08-25 08:07:51: File Deleted - 384094: nijtmans added on 2010-08-25 07:50:07: Joe English wrote on Tcl List Core <[email protected]> > New stub entries are not in fact required here. > We only need to worry about ABI compatibility for platforms > on which binaries currently actually exist. 'char *' and 'void *' > (and const- and volatile- qualified versions of same) have identical > representations on all currently known supported platforms. It turns out that he is right. See: <http://bytes.com/topic/c/answers/221528-function-called-through-non-compatible-type> > Cray vector machines use a different representation for char* and > void* pointers than for int* pointers. A native machine address > points to a 64-bit word; byte pointers (implemented in software) store > a 3-bit offset in the unused high-order bits. The representation is > such that an arbitrary int* pointer happens to be a valid byte pointer > to the first byte of the word (the offset happens to be zero). So, I will put back the stub entry in the original location. nijtmans added on 2010-08-21 05:13:44: File Added - 384094: fs2.patch nijtmans added on 2010-08-21 05:11:36: allow_comments - 0 There is one little thing left: binary compatibily. Because of the moved Tcl_FSGetNativePath to a different stub entry, Extensions using this function compiled with Tcl 8.6 will no longer run in earlier Tcl versions. This is called "forwards compatibility", and in generality this is not promised by Tcl, but still a good thing, the more when the solution is rather simple. Here is a patch for Tcl 8.5 that adds a dummy entry to the stub table, that cannot be called in any way by Tcl 8.5 extensions. It restores this "forward compatibility" without any side effects dgp added on 2010-08-16 21:49:02: allow_comments - 1 The change you've made with this patch looks good to me. It takes some effort to see that because the change is near a lot of confusion and messiness in the Tcl_Filesystem. We have two ways to get a driver-specific rep for a filesystem path: Tcl_FSGetNativePath() Tcl_FSGetInternalRep() and we have one way to set Tcl_FSNewNativePath() By the names, you would think that GetNativePath and NewNativePath were a corresponding pair, but in fact it is NewNativePath and GetInterpRep that go together and offer the way to interact with the "native" or "internal" rep used by any Tcl_Filesystem at all. Tcl_FSGetNativePath is the oddball, specialized convenience interface useful only for the one Tcl_Filesystem declared to be "native" in the current app. Since it's out on its own, this patch is a good change, and one apparently suggested in the docs for some time. If GetNativePath actually were a pair with NewNativePath, I would insist they agree and operated on ClientData. It takes some effort to see that is not the case. Anyhow, I think this patch is good for Tcl 8.6. Thanks! nijtmans added on 2010-04-29 20:18:57: File Added - 372236: fs.patch |
Attachments:
- fs.patch [download] added by nijtmans on 2010-04-29 20:18:57. [details]