Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use c_char instead of i8 to compile on platforms where c_char = u8 #6572

Merged
merged 1 commit into from
Oct 17, 2024
Merged

Use c_char instead of i8 to compile on platforms where c_char = u8 #6572

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions arrow-integration-testing/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use arrow::record_batch::RecordBatch;
use arrow::util::test_util::arrow_test_data;
use arrow_integration_test::*;
use std::collections::HashMap;
use std::ffi::{c_int, CStr, CString};
use std::ffi::{c_char, c_int, CStr, CString};
use std::fs::File;
use std::io::BufReader;
use std::iter::zip;
Expand Down Expand Up @@ -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) };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Screenshot 2024-10-16 at 3 20 24 PM

Maybe it changed in 1.82 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rust-lang/rust#113735

Expand All @@ -178,7 +178,7 @@ fn cdata_integration_export_schema_from_json(

/// C Data Integration entrypoint to export a batch from a JSON file
fn cdata_integration_export_batch_from_json(
c_json_name: *const i8,
c_json_name: *const c_char,
batch_num: c_int,
out: *mut FFI_ArrowArray,
) -> Result<()> {
Expand All @@ -192,7 +192,7 @@ fn cdata_integration_export_batch_from_json(
}

fn cdata_integration_import_schema_and_compare_to_json(
c_json_name: *const i8,
c_json_name: *const c_char,
c_schema: *mut FFI_ArrowSchema,
) -> Result<()> {
let json_name = unsafe { CStr::from_ptr(c_json_name) };
Expand Down Expand Up @@ -229,7 +229,7 @@ fn compare_batches(a: &RecordBatch, b: &RecordBatch) -> Result<()> {
}

fn cdata_integration_import_batch_and_compare_to_json(
c_json_name: *const i8,
c_json_name: *const c_char,
batch_num: c_int,
c_array: *mut FFI_ArrowArray,
) -> Result<()> {
Expand All @@ -248,7 +248,7 @@ fn cdata_integration_import_batch_and_compare_to_json(
}

// If Result is an error, then export a const char* to its string display, otherwise NULL
fn result_to_c_error<T, E: std::fmt::Display>(result: &std::result::Result<T, E>) -> *mut i8 {
fn result_to_c_error<T, E: std::fmt::Display>(result: &std::result::Result<T, E>) -> *mut c_char {
match result {
Ok(_) => ptr::null_mut(),
Err(e) => CString::new(format!("{}", e)).unwrap().into_raw(),
Expand All @@ -261,7 +261,7 @@ fn result_to_c_error<T, E: std::fmt::Display>(result: &std::result::Result<T, E>
///
/// The pointer is assumed to have been obtained using CString::into_raw.
#[no_mangle]
pub unsafe extern "C" fn arrow_rs_free_error(c_error: *mut i8) {
pub unsafe extern "C" fn arrow_rs_free_error(c_error: *mut c_char) {
if !c_error.is_null() {
drop(unsafe { CString::from_raw(c_error) });
}
Expand All @@ -270,41 +270,41 @@ pub unsafe extern "C" fn arrow_rs_free_error(c_error: *mut i8) {
/// A C-ABI for exporting an Arrow schema from a JSON file
#[no_mangle]
pub extern "C" fn arrow_rs_cdata_integration_export_schema_from_json(
c_json_name: *const i8,
c_json_name: *const c_char,
out: *mut FFI_ArrowSchema,
) -> *mut i8 {
) -> *mut c_char {
let r = cdata_integration_export_schema_from_json(c_json_name, out);
result_to_c_error(&r)
}

/// A C-ABI to compare an Arrow schema against a JSON file
#[no_mangle]
pub extern "C" fn arrow_rs_cdata_integration_import_schema_and_compare_to_json(
c_json_name: *const i8,
c_json_name: *const c_char,
c_schema: *mut FFI_ArrowSchema,
) -> *mut i8 {
) -> *mut c_char {
let r = cdata_integration_import_schema_and_compare_to_json(c_json_name, c_schema);
result_to_c_error(&r)
}

/// A C-ABI for exporting a RecordBatch from a JSON file
#[no_mangle]
pub extern "C" fn arrow_rs_cdata_integration_export_batch_from_json(
c_json_name: *const i8,
c_json_name: *const c_char,
batch_num: c_int,
out: *mut FFI_ArrowArray,
) -> *mut i8 {
) -> *mut c_char {
let r = cdata_integration_export_batch_from_json(c_json_name, batch_num, out);
result_to_c_error(&r)
}

/// A C-ABI to compare a RecordBatch against a JSON file
#[no_mangle]
pub extern "C" fn arrow_rs_cdata_integration_import_batch_and_compare_to_json(
c_json_name: *const i8,
c_json_name: *const c_char,
batch_num: c_int,
c_array: *mut FFI_ArrowArray,
) -> *mut i8 {
) -> *mut c_char {
let r = cdata_integration_import_batch_and_compare_to_json(c_json_name, batch_num, c_array);
result_to_c_error(&r)
}
Loading