Use standard Julia mechanisms for LibGit2 StrArrayStruct and Buffer types. #19962
Conversation
| end | ||
| return arr | ||
| function Base.unsafe_convert(::Type{Ptr{StrArrayStruct}}, tup::Tuple) | ||
| Base.unsafe_convert(Ptr{StrArrayStruct}, first(tup)) |
There was a problem hiding this comment.
why does this throw away all but the first element?
| ``` | ||
| In particular, note that `LibGit2.free` should be called afterward on the `Ref` object. | ||
|
|
||
| Conversely, when a vector of strings to LibGit2, it is generally simplest to rely on |
| data = [Base.cconvert(Cstring, s) for s in strs] | ||
| ptrs = [Base.unsafe_convert(Cstring, d) for d in data] | ||
| sa = Ref(StrArrayStruct(pointer(ptrs), length(ptrs))) | ||
| sa, data, ptrs |
There was a problem hiding this comment.
@tkelman related to your comment below: the idea is that cconvert returns a tuple but maintains a reference to the original data.
I agree it's not very pretty: any ideas for a better way to do it?
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Really pedantic point, but perhaps 4 line breaks is an unnecessary amount of vertical space?
| @kwdef immutable StrArrayStruct | ||
|
|
||
| When fetching data from LibGit2, a typical usage would look like: | ||
| ``` |
There was a problem hiding this comment.
```julia? If that seems a worthwhile change, it could be applied to other blocks in the docstrings as well.
| end | ||
| return arr | ||
| function Base.cconvert(::Type{Ptr{StrArrayStruct}}, x::Vector) | ||
| str_ref = Base.cconvert(Ref{Cstring}, x) |
There was a problem hiding this comment.
@vtjnash Does this look okay?
| (Ptr{StrArrayStruct}, Ptr{StrArrayStruct}), | ||
| dst_ptr, Ref(src)) | ||
| return dst_ptr[] | ||
| function Base.unsafe_convert(::Type{Vector{String}}, sa::StrArrayStruct) |
There was a problem hiding this comment.
I think this is simply convert, no unsafe_ prefix necessary.
|
Okay, I'm fairly happy with this now. Thanks everyone. |
The code currently calls
Libc.malloc, and then relies on the libraryfunction to free the data. This seems slightly dubious, so I rejigged it
to use
cconvertandunsafe_convert.