Tcl Source Code

View Ticket
Login
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: