Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use libuv for tempdir function #31434

Merged
merged 2 commits into from
Jun 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 24 additions & 18 deletions base/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -424,18 +424,34 @@ function touch(path::AbstractString)
path
end

const temp_prefix = "jl_"

if Sys.iswindows()
"""
tempdir()

Gets the path of the temporary directory. On Windows, `tempdir()` uses the first environment
variable found in the ordered list `TMP`, `TEMP`, `USERPROFILE`. On all other operating
systems, `tempdir()` uses the first environment variable found in the ordered list `TMPDIR`,
`TMP`, `TEMP`, and `TEMPDIR`. If none of these are found, the path `"/tmp"` is used.
"""
function tempdir()
temppath = Vector{UInt16}(undef, 32767)
lentemppath = ccall(:GetTempPathW, stdcall, UInt32, (UInt32, Ptr{UInt16}), length(temppath), temppath)
windowserror("GetTempPath", lentemppath >= length(temppath) || lentemppath == 0)
resize!(temppath, lentemppath)
return transcode(String, temppath)
buf = Base.StringVector(AVG_PATH - 1) # space for null-terminator implied by StringVector
sz = RefValue{Csize_t}(length(buf) + 1) # total buffer size including null
while true
rc = ccall(:uv_os_tmpdir, Cint, (Ptr{UInt8}, Ptr{Csize_t}), buf, sz)
if rc == 0
resize!(buf, sz[])
return String(buf)
elseif rc == Base.UV_ENOBUFS
resize!(buf, sz[] - 1) # space for null-terminator implied by StringVector
else
uv_error(:tmpdir, rc)
end
end
end

const temp_prefix = "jl_"

if Sys.iswindows()

function _win_tempname(temppath::AbstractString, uunique::UInt32)
tempp = cwstring(temppath)
temppfx = cwstring(temp_prefix)
Expand Down Expand Up @@ -481,9 +497,6 @@ function tempname()
return s
end
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to use tempdir() for d?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will make these changes in a separate PR.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm. feel free to merge.


# Obtain a temporary directory's path.
tempdir() = dirname(tempname())

# Create and return the name of a temporary file along with an IOStream
function mktemp(parent=tempdir())
b = joinpath(parent, temp_prefix * "XXXXXX")
Expand All @@ -496,13 +509,6 @@ end
end # os-test


"""
tempdir()

Obtain the path of a temporary directory (possibly shared with other processes).
"""
tempdir()

"""
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the tempname doc string below, it might be clearer to add, "This can lead to security holes." and recommend opening the file with O_EXCL, if this is a concern (in addition to the suggestion to use mktemp).

Aside: the windows implementation of mktemp appears to be wrong because it fails to heed this warning.

tempname()

Expand Down
35 changes: 33 additions & 2 deletions test/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,39 @@ close(s)
end
end

my_tempdir = tempdir()
@test isdir(my_tempdir) == true
@testset "tempdir" begin
my_tempdir = tempdir()
@test isdir(my_tempdir)
@test my_tempdir[end] != '/'
@test my_tempdir[end] != '\\'

var = Sys.iswindows() ? "TMP" : "TMPDIR"
PATH_PREFIX = Sys.iswindows() ? "C:\\" : "/tmp/"
# Warning: On Windows uv_os_tmpdir internally calls GetTempPathW. The max string length for
# GetTempPathW is 261 (including the implied trailing backslash), not the typical length 259.
# We thus use 260 (with implied trailing slash backlash this then gives 261 chars) and
# subtract 9 to account for i = 0:9.
MAX_PATH = (Sys.iswindows() ? 260-9 : 1024) - length(PATH_PREFIX)
for i = 0:8
local tmp = PATH_PREFIX * "x"^MAX_PATH * "123456789"[1:i]
@test withenv(var => tmp) do
tempdir()
end == (tmp)
end
for i = 9
local tmp = PATH_PREFIX * "x"^MAX_PATH * "123456789"[1:i]
if Sys.iswindows()
# libuv bug
@test_broken withenv(var => tmp) do
tempdir()
end == tmp
else
@test withenv(var => tmp) do
tempdir()
end == tmp
end
end
end

let path = tempname()
# issue #9053
Expand Down