Use c_char instead of i8 to compile on platforms where c_char = u8 #6572
Conversation
|
Fascinating -- it appears Maybe this is some sort of foreshadowing of what is to come |
| @@ -165,7 +165,7 @@ pub fn read_gzip_json(version: &str, path: &str) -> ArrowJson { | |||
|
|
|||
| /// C Data Integration entrypoint to export the schema from a JSON file | |||
| fn cdata_integration_export_schema_from_json( | |||
| c_json_name: *const i8, | |||
| c_json_name: *const c_char, | |||
| out: *mut FFI_ArrowSchema, | |||
| ) -> Result<()> { | |||
| let json_name = unsafe { CStr::from_ptr(c_json_name) }; | |||
There was a problem hiding this comment.
It is strange to me that the signature in std::ffi is ptr: *const i8, not *const c_char 🤔
https://doc.rust-lang.org/std/ffi/struct.CStr.html#method.from_ptr
Maybe it changed in 1.82 🤔
There was a problem hiding this comment.
So it does seem to say *const i8 in the signature, but if you click through to the source, it shows ptr: *const c_char. I don't know what's up with that, it feels like a docs.rs bug.
There was a problem hiding this comment.
Hmmm, if you cd ~/.rustup/toolchains/stable-*/lib/rustlib/src/rust/library/core && RUSTC_BOOTSTRAP=1 cargo +stable doc --open, it shows ptr: *const c_char for this fn, which is correct. I'm going to try to figure out if someone else has reported this.
There was a problem hiding this comment.
There was a problem hiding this comment.
Thank you @itsjunetime -- this makes sense to me
the CI failure https://github.com/apache/arrow-rs/actions/runs/11370034366/job/31636443148?pr=6572 on this PR is unrelated -- see #6568
|
CI Integration failure is tracked with #6577 This PR basically uses a different typedef, so I don't think it is a breaking change Thus 🚀 |
|
Thanks again @itsjunetime and @mbrobbel |
Which issue does this PR close?
Closes #6571
Rationale for this change
c_charis what is actually required for these APIs, so we should uniformly use it where required.What changes are included in this PR?
Switches a bunch of
i8s toc_charsAre there any user-facing changes?
No