-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
RFC: Rewrite homedir based on uv_os_homedir #19636
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And a better implementation is
julia> function homedir2()
sz = Ref{Csize_t}(1024 + 1)
buf = Vector{UInt8}(sz[] - 1)
while true
rc = ccall(:uv_os_homedir, Cint, (Ptr{UInt8}, Ptr{Csize_t}), buf, sz)
if rc == 0
resize!(buf, sz[])
return String(buf)
elseif rc == Base.UV_ENOBUFS # No Base. in base
resize!(buf, sz[] - 1)
else
error("Unable to retrieve home directory.")
end
end
end
buf = Vector{Cchar}(1024) | ||
sz = Ref{Csize_t}(sizeof(buf)) | ||
rc = ccall(:uv_os_homedir, Cint, (Ptr{Cchar}, Ptr{Csize_t}), buf, sz) | ||
rc == 0 || error("unable to retrieve home directory") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard to imagine it will happen but the correct way is to use sz[] - 1
to resize the array.
homedir() | ||
function homedir() | ||
buf = Vector{Cchar}(1024) | ||
sz = Ref{Csize_t}(sizeof(buf)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be sizeof(buf) + 1
rc = ccall(:uv_os_homedir, Cint, (Ptr{Cchar}, Ptr{Csize_t}), buf, sz) | ||
rc == 0 || error("unable to retrieve home directory") | ||
nbytes = Int(sz[]) | ||
return unsafe_string(pointer(buf[1:nbytes])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong and inefficient.
Thanks for the feedback, @yuyichao!
|
That s why I used UInt8 |
Oh sorry! Missed that part. |
Looking better, @yuyichao? |
With this change, is there any way to override the home directory Julia uses? I'd like my Julia homedir to be the home directory under my MSYS2/MinGW installation instead of the standard user dir Windows defines. |
@asampal, yes. From the libuv documentation:
So, setting
|
@stevengj if I redefine |
Fixes #11331
Rather than relying on environment variables to determine the user's home directory, this calls
uv_os_homedir
from libuv, per @stevengj's suggestion.The "magic number" of 1024 in there is what we're setting as the
PATH_MAX
if none is found in the system (see src/support/dirpath.h), so it seemed to me like a decent choice for the number of bytes to allocate for the output path. Let me know if that isn't reasonable or if there's a better choice.For reference, here is the upstream libuv documentation on
uv_os_homedir
: http://docs.libuv.org/en/v1.x/misc.html#c.uv_os_homedir