Skip to content

Commit

Permalink
Fix wrong pointer type of static _dispatch_main_q binding
Browse files Browse the repository at this point in the history
While looking at a copy of this code (in another project that uses
`objc2`, which does not yet define `dispatch` integration) this strange
cast immediately stands out.  `_dispatch_main_q` is defined here as a
`dispatch_queue_t`, the `queue` argument in `dispatch_data_create()`
is also defined as a `dispatch_queue_t`, yet there is a "borrow" (get
*pointer to*) operation before passing it into the function.

Instead, this `static` field is supposed to be defined as the
`Object` itself, so that any user could take a _pointer to it_
if they wish or need it so.  The same is seen in [the `dispatch`
crate]: `_dispatch_main_q` is defined as a `dispatch_object_s`, and
`dispatch_queue_t` is a _typedef_ to a `*mut dispatch_object_s` to match
the above convention.

Note that this is not a bugfix, but merely a readability improvement.
Use of `mut` here is debatable.

[the `dispatch` crate]: https://github.com/SSheldon/rust-dispatch/blob/f540a2d8ccaebf0e87f5805033b9e287e8d01ba5/src/ffi.rs#L33
  • Loading branch information
MarijnS95 committed Jan 2, 2025
1 parent ef768ff commit 4d0aa6b
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions src/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1475,16 +1475,22 @@ type dispatch_block_t = *const Block<(), ()>;
const DISPATCH_DATA_DESTRUCTOR_DEFAULT: dispatch_block_t = ptr::null();

#[cfg_attr(
all(feature = "link", any(target_os = "macos", target_os = "ios", target_os = "visionos")),
all(
feature = "link",
any(target_os = "macos", target_os = "ios", target_os = "visionos")
),
link(name = "System", kind = "dylib")
)]
#[cfg_attr(
all(feature = "link", not(any(target_os = "macos", target_os = "ios", target_os = "visionos"))),
all(
feature = "link",
not(any(target_os = "macos", target_os = "ios", target_os = "visionos"))
),
link(name = "dispatch", kind = "dylib")
)]
#[allow(improper_ctypes)]
extern "C" {
static _dispatch_main_q: dispatch_queue_t;
static mut _dispatch_main_q: Object;

fn dispatch_data_create(
buffer: *const std::ffi::c_void,
Expand Down Expand Up @@ -1719,7 +1725,7 @@ impl DeviceRef {
let data = dispatch_data_create(
library_data.as_ptr() as *const std::ffi::c_void,
library_data.len() as crate::c_size_t,
&_dispatch_main_q as *const _ as dispatch_queue_t,
&mut _dispatch_main_q,
DISPATCH_DATA_DESTRUCTOR_DEFAULT,
);

Expand Down

0 comments on commit 4d0aa6b

Please sign in to comment.