Skip to content

Commit 8e0183f

Browse files
authored
filesystem: code cleanup in request calls (#39012)
The canonical way we allocate the libuv requests in the filesystem API is through malloc/free calls. Clean up the code to be more DRY.
1 parent 187f96b commit 8e0183f

File tree

3 files changed

+34
-28
lines changed

3 files changed

+34
-28
lines changed

base/file.jl

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -175,10 +175,10 @@ function mkdir(path::AbstractString; mode::Integer = 0o777)
175175
(Ptr{Cvoid}, Ptr{Cvoid}, Cstring, Cint, Ptr{Cvoid}),
176176
C_NULL, req, path, checkmode(mode), C_NULL)
177177
if ret < 0
178-
ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req)
178+
uv_fs_req_cleanup(req)
179179
uv_error("mkdir($(repr(path)); mode=0o$(string(mode,base=8)))", ret)
180180
end
181-
ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req)
181+
uv_fs_req_cleanup(req)
182182
return path
183183
finally
184184
Libc.free(req)
@@ -678,11 +678,11 @@ function mktempdir(parent::AbstractString=tempdir();
678678
(Ptr{Cvoid}, Ptr{Cvoid}, Cstring, Ptr{Cvoid}),
679679
C_NULL, req, tpath, C_NULL)
680680
if ret < 0
681-
ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req)
681+
uv_fs_req_cleanup(req)
682682
uv_error("mktempdir($(repr(parent)))", ret)
683683
end
684684
path = unsafe_string(ccall(:jl_uv_fs_t_path, Cstring, (Ptr{Cvoid},), req))
685-
ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req)
685+
uv_fs_req_cleanup(req)
686686
cleanup && temp_cleanup_later(path)
687687
return path
688688
finally
@@ -822,28 +822,31 @@ julia> readdir(abspath("base"), join=true)
822822
"""
823823
function readdir(dir::AbstractString; join::Bool=false, sort::Bool=true)
824824
# Allocate space for uv_fs_t struct
825-
uv_readdir_req = zeros(UInt8, ccall(:jl_sizeof_uv_fs_t, Int32, ()))
826-
827-
# defined in sys.c, to call uv_fs_readdir, which sets errno on error.
828-
err = ccall(:uv_fs_scandir, Int32, (Ptr{Cvoid}, Ptr{UInt8}, Cstring, Cint, Ptr{Cvoid}),
829-
C_NULL, uv_readdir_req, dir, 0, C_NULL)
830-
err < 0 && uv_error("readdir($(repr(dir)))", err)
831-
832-
# iterate the listing into entries
833-
entries = String[]
834-
ent = Ref{uv_dirent_t}()
835-
while Base.UV_EOF != ccall(:uv_fs_scandir_next, Cint, (Ptr{Cvoid}, Ptr{uv_dirent_t}), uv_readdir_req, ent)
836-
name = unsafe_string(ent[].name)
837-
push!(entries, join ? joinpath(dir, name) : name)
838-
end
825+
req = Libc.malloc(_sizeof_uv_fs)
826+
try
827+
# defined in sys.c, to call uv_fs_readdir, which sets errno on error.
828+
err = ccall(:uv_fs_scandir, Int32, (Ptr{Cvoid}, Ptr{Cvoid}, Cstring, Cint, Ptr{Cvoid}),
829+
C_NULL, req, dir, 0, C_NULL)
830+
err < 0 && uv_error("readdir($(repr(dir)))", err)
831+
832+
# iterate the listing into entries
833+
entries = String[]
834+
ent = Ref{uv_dirent_t}()
835+
while Base.UV_EOF != ccall(:uv_fs_scandir_next, Cint, (Ptr{Cvoid}, Ptr{uv_dirent_t}), req, ent)
836+
name = unsafe_string(ent[].name)
837+
push!(entries, join ? joinpath(dir, name) : name)
838+
end
839839

840-
# Clean up the request string
841-
ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{UInt8},), uv_readdir_req)
840+
# Clean up the request string
841+
uv_fs_req_cleanup(req)
842842

843-
# sort entries unless opted out
844-
sort && sort!(entries)
843+
# sort entries unless opted out
844+
sort && sort!(entries)
845845

846-
return entries
846+
return entries
847+
finally
848+
Libc.free(req)
849+
end
847850
end
848851
readdir(; join::Bool=false, sort::Bool=true) =
849852
readdir(join ? pwd() : ".", join=join, sort=sort)
@@ -1019,12 +1022,12 @@ function readlink(path::AbstractString)
10191022
(Ptr{Cvoid}, Ptr{Cvoid}, Cstring, Ptr{Cvoid}),
10201023
C_NULL, req, path, C_NULL)
10211024
if ret < 0
1022-
ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req)
1025+
uv_fs_req_cleanup(req)
10231026
uv_error("readlink($(repr(path)))", ret)
10241027
@assert false
10251028
end
10261029
tgt = unsafe_string(ccall(:jl_uv_fs_t_ptr, Cstring, (Ptr{Cvoid},), req))
1027-
ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req)
1030+
uv_fs_req_cleanup(req)
10281031
return tgt
10291032
finally
10301033
Libc.free(req)

base/filesystem.jl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ end
5454
# On Windows we use the MAX_PATH = 260 value on Win32.
5555
const AVG_PATH = Sys.iswindows() ? 260 : 512
5656

57+
# helper function to clean up libuv request
58+
uv_fs_req_cleanup(req) = ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req)
59+
5760
include("path.jl")
5861
include("stat.jl")
5962
include("file.jl")
@@ -83,7 +86,7 @@ function open(path::AbstractString, flags::Integer, mode::Integer=0)
8386
(Ptr{Cvoid}, Ptr{Cvoid}, Cstring, Int32, Int32, Ptr{Cvoid}),
8487
C_NULL, req, path, flags, mode, C_NULL)
8588
handle = ccall(:uv_fs_get_result, Cssize_t, (Ptr{Cvoid},), req)
86-
ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req)
89+
uv_fs_req_cleanup(req)
8790
ret < 0 && uv_error("open($(repr(path)), $flags, $mode)", ret)
8891
finally # conversion to Cstring could cause an exception
8992
Libc.free(req)

base/path.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -435,11 +435,11 @@ function realpath(path::AbstractString)
435435
(Ptr{Cvoid}, Ptr{Cvoid}, Cstring, Ptr{Cvoid}),
436436
C_NULL, req, path, C_NULL)
437437
if ret < 0
438-
ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req)
438+
uv_fs_req_cleanup(req)
439439
uv_error("realpath($(repr(path)))", ret)
440440
end
441441
path = unsafe_string(ccall(:jl_uv_fs_t_ptr, Cstring, (Ptr{Cvoid},), req))
442-
ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req)
442+
uv_fs_req_cleanup(req)
443443
return path
444444
finally
445445
Libc.free(req)

0 commit comments

Comments
 (0)