Skip to content

unsafe_string should accept both UInt8 and Int8 pointer type #244

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

Closed
wants to merge 1 commit into from

Conversation

r9y9
Copy link

@r9y9 r9y9 commented Jul 1, 2016

As Julia master does:

julia> methods(unsafe_string)
#3 methods for generic function "unsafe_string":
unsafe_string(s::Cstring) at c.jl:69
unsafe_string(p::Union{Ptr{Int8},Ptr{UInt8}}) at strings/basic.jl:56
unsafe_string(p::Union{Ptr{Int8},Ptr{UInt8}}, len::Integer) at strings/basic.jl:52

julia> versioninfo()
Julia Version 0.5.0-dev+5063
Commit 8ecd657* (2016-07-01 04:21 UTC)
Platform Info:
  System: Darwin (x86_64-apple-darwin15.5.0)
  CPU: Intel(R) Core(TM) i5-4258U CPU @ 2.40GHz
  WORD_SIZE: 64
  BLAS: libopenblas (USE64BITINT DYNAMIC_ARCH NO_AFFINITY Haswell)
  LAPACK: libopenblas64_
  LIBM: libopenlibm
  LLVM: libLLVM-3.7.1 (ORCJIT, haswell)

Without this, e.g. the following doesn't work (on v0.4.5 at least):

using Compat
unsafe_string(ccall((:getenv, "libc"), Ptr{Cchar}, (Ptr{UInt8},),
            Compat.unsafe_convert(Ptr{UInt8}, "HOME")))
LoadError: MethodError: `unsafe_string` has no method matching unsafe_string(::Ptr{Int8})
while loading In[28], in expression starting on line 1

when Cchar is aliased to Int8.

Without this, e.g. the following doesn't work:

```jl
using Compat
unsafe_string(ccall((:getenv, "libc"), Ptr{Cchar}, (Ptr{UInt8},),
            Compat.unsafe_convert(Ptr{UInt8}, "HOME")))
```

when Cchar is aliased to Int8.

Also trailing whitespaces
@stevengj
Copy link
Member

stevengj commented Jul 1, 2016

You need @compat to use Union{...}, since that syntax won't work on 0.3. Also, unsafe_wrap needs the same fix.

I think #245 should supersede this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants