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 780f9b5
Showing 1 changed file with 16 additions and 5 deletions.
21 changes: 16 additions & 5 deletions src/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ use block::Block;
use log::warn;
use objc::runtime::{NO, YES};

use std::{ffi::CStr, os::raw::c_char, path::Path, ptr};
use std::{
ffi::CStr,
os::raw::c_char,
path::Path,
ptr::{self, addr_of_mut},
};

/// Available on macOS 10.11+, iOS 8.0+, tvOS 9.0+
///
Expand Down Expand Up @@ -1475,16 +1480,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 +1730,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,
addr_of_mut!(_dispatch_main_q),
DISPATCH_DATA_DESTRUCTOR_DEFAULT,
);

Expand Down

0 comments on commit 780f9b5

Please sign in to comment.