Skip to content

Commit

Permalink
Don't use as for casting pointers (only for *const T -> *mut T)
Browse files Browse the repository at this point in the history
Easy to accidentally convert immutable pointers to a mutable ones, as is done in `declare` (now fixed).
  • Loading branch information
madsmtm committed Jun 12, 2022
1 parent 9151c50 commit b8f9b88
Show file tree
Hide file tree
Showing 14 changed files with 129 additions and 100 deletions.
17 changes: 10 additions & 7 deletions objc2-foundation/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,12 @@ impl NSData {

pub fn bytes(&self) -> &[u8] {
let ptr: *const c_void = unsafe { msg_send![self, bytes] };
let ptr: *const u8 = ptr.cast();
// The bytes pointer may be null for length zero
if ptr.is_null() {
&[]
} else {
unsafe { slice::from_raw_parts(ptr as *const u8, self.len()) }
unsafe { slice::from_raw_parts(ptr, self.len()) }
}
}

Expand Down Expand Up @@ -172,11 +173,12 @@ impl NSMutableData {
#[doc(alias = "mutableBytes")]
pub fn bytes_mut(&mut self) -> &mut [u8] {
let ptr = self.raw_bytes_mut();
let ptr: *mut u8 = ptr.cast();
// The bytes pointer may be null for length zero
if ptr.is_null() {
&mut []
} else {
unsafe { slice::from_raw_parts_mut(ptr as *mut u8, self.len()) }
unsafe { slice::from_raw_parts_mut(ptr, self.len()) }
}
}

Expand All @@ -188,7 +190,7 @@ impl NSMutableData {

#[doc(alias = "appendBytes:length:")]
pub fn extend_from_slice(&mut self, bytes: &[u8]) {
let bytes_ptr = bytes.as_ptr() as *const c_void;
let bytes_ptr: *const c_void = bytes.as_ptr().cast();
unsafe { msg_send![self, appendBytes: bytes_ptr, length: bytes.len()] }
}

Expand All @@ -201,7 +203,7 @@ impl NSMutableData {
let range = NSRange::from(range);
// No need to verify the length of the range here,
// `replaceBytesInRange:` just zero-fills if out of bounds.
let ptr = bytes.as_ptr() as *const c_void;
let ptr: *const c_void = bytes.as_ptr().cast();
unsafe {
msg_send![
self,
Expand Down Expand Up @@ -312,7 +314,7 @@ impl DefaultId for NSMutableData {
}

unsafe fn data_with_bytes(cls: &Class, bytes: &[u8]) -> *mut Object {
let bytes_ptr = bytes.as_ptr() as *const c_void;
let bytes_ptr: *const c_void = bytes.as_ptr().cast();
unsafe {
let obj: *mut Object = msg_send![cls, alloc];
msg_send![
Expand All @@ -333,18 +335,19 @@ unsafe fn data_from_vec(cls: &Class, bytes: Vec<u8>) -> *mut Object {

let dealloc = ConcreteBlock::new(move |bytes: *mut c_void, len: usize| unsafe {
// Recreate the Vec and let it drop
let _ = Vec::from_raw_parts(bytes as *mut u8, len, capacity);
let _ = Vec::<u8>::from_raw_parts(bytes.cast(), len, capacity);
});
let dealloc = dealloc.copy();
let dealloc: &Block<(*mut c_void, usize), ()> = &dealloc;

let mut bytes = ManuallyDrop::new(bytes);
let bytes_ptr: *mut c_void = bytes.as_mut_ptr().cast();

unsafe {
let obj: *mut Object = msg_send![cls, alloc];
msg_send![
obj,
initWithBytesNoCopy: bytes.as_mut_ptr() as *mut c_void,
initWithBytesNoCopy: bytes_ptr,
length: bytes.len(),
deallocator: dealloc,
]
Expand Down
8 changes: 4 additions & 4 deletions objc2-foundation/src/enumerator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,25 +172,25 @@ mod tests {

#[test]
fn test_enumerator() {
let vec = (0u32..4).map(NSValue::new).collect();
let vec = (0usize..4).map(NSValue::new).collect();
let array = NSArray::from_vec(vec);

let enumerator = array.iter();
assert_eq!(enumerator.count(), 4);

let enumerator = array.iter();
assert!(enumerator.enumerate().all(|(i, obj)| obj.get() == i as u32));
assert!(enumerator.enumerate().all(|(i, obj)| obj.get() == i));
}

#[test]
fn test_fast_enumerator() {
let vec = (0u32..4).map(NSValue::new).collect();
let vec = (0usize..4).map(NSValue::new).collect();
let array = NSArray::from_vec(vec);

let enumerator = array.iter_fast();
assert_eq!(enumerator.count(), 4);

let enumerator = array.iter_fast();
assert!(enumerator.enumerate().all(|(i, obj)| obj.get() == i as u32));
assert!(enumerator.enumerate().all(|(i, obj)| obj.get() == i));
}
}
4 changes: 2 additions & 2 deletions objc2-foundation/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl NSString {
//
// https://developer.apple.com/documentation/foundation/nsstring/1411189-utf8string?language=objc
let bytes: *const c_char = unsafe { msg_send![self, UTF8String] };
let bytes = bytes as *const u8;
let bytes: *const u8 = bytes.cast();
let len = self.len();

// SAFETY:
Expand Down Expand Up @@ -199,7 +199,7 @@ impl NSString {
}

pub(crate) fn from_str(cls: &Class, string: &str) -> *mut Object {
let bytes = string.as_ptr() as *const c_void;
let bytes: *const c_void = string.as_ptr().cast();
unsafe {
let obj: *mut Object = msg_send![cls, alloc];
msg_send![
Expand Down
5 changes: 3 additions & 2 deletions objc2-foundation/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl<T: 'static + Copy + Encode> NSValue<T> {
/// The user must ensure that the inner value is properly initialized.
pub unsafe fn get_unchecked(&self) -> T {
let mut value = MaybeUninit::<T>::uninit();
let ptr = value.as_mut_ptr() as *mut c_void;
let ptr: *mut c_void = value.as_mut_ptr().cast();
let _: () = unsafe { msg_send![self, getValue: ptr] };
unsafe { value.assume_init() }
}
Expand All @@ -64,7 +64,8 @@ impl<T: 'static + Copy + Encode> NSValue<T> {

pub fn new(value: T) -> Id<Self, Shared> {
let cls = Self::class();
let bytes = &value as *const T as *const c_void;
let bytes: *const T = &value;
let bytes: *const c_void = bytes.cast();
let encoding = CString::new(T::ENCODING.to_string()).unwrap();
unsafe {
let obj: *mut Self = msg_send![cls, alloc];
Expand Down
3 changes: 2 additions & 1 deletion objc2/examples/talk_to_me.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ fn main() {
const UTF8_ENCODING: NSUInteger = 4;

let string: *const Object = unsafe { msg_send![class!(NSString), alloc] };
let text_ptr: *const c_void = text.as_ptr().cast();
let string = unsafe {
msg_send![
string,
initWithBytes: text.as_ptr() as *const c_void,
initWithBytes: text_ptr,
length: text.len(),
encoding: UTF8_ENCODING,
]
Expand Down
10 changes: 5 additions & 5 deletions objc2/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ impl CachedSel {
// `Relaxed` should be fine since `sel_registerName` is thread-safe.
let ptr = self.ptr.load(Ordering::Relaxed);
if ptr.is_null() {
let ptr = unsafe { ffi::sel_registerName(name.as_ptr() as *const _) };
self.ptr.store(ptr as *mut _, Ordering::Relaxed);
unsafe { Sel::from_ptr(ptr as *const _) }
let ptr: *const c_void = unsafe { ffi::sel_registerName(name.as_ptr().cast()).cast() };
self.ptr.store(ptr as *mut c_void, Ordering::Relaxed);
unsafe { Sel::from_ptr(ptr) }
} else {
unsafe { Sel::from_ptr(ptr) }
}
Expand Down Expand Up @@ -58,8 +58,8 @@ impl CachedClass {
// `Relaxed` should be fine since `objc_getClass` is thread-safe.
let ptr = self.ptr.load(Ordering::Relaxed);
if ptr.is_null() {
let cls = unsafe { ffi::objc_getClass(name.as_ptr() as *const _) } as *const Class;
self.ptr.store(cls as *mut _, Ordering::Relaxed);
let cls: *const Class = unsafe { ffi::objc_getClass(name.as_ptr().cast()) }.cast();
self.ptr.store(cls as *mut Class, Ordering::Relaxed);
unsafe { cls.as_ref() }
} else {
Some(unsafe { &*ptr })
Expand Down
45 changes: 24 additions & 21 deletions objc2/src/declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,13 @@ unsafe impl Send for ClassBuilder {}
unsafe impl Sync for ClassBuilder {}

impl ClassBuilder {
fn as_ptr(&self) -> *mut ffi::objc_class {
fn as_mut_ptr(&mut self) -> *mut ffi::objc_class {
self.cls.as_ptr().cast()
}

fn with_superclass(name: &str, superclass: Option<&Class>) -> Option<Self> {
let name = CString::new(name).unwrap();
let super_ptr = superclass.map_or(ptr::null(), |c| c) as _;
let super_ptr = superclass.map_or(ptr::null(), |c| c).cast();
let cls = unsafe { ffi::objc_allocateClassPair(super_ptr, name.as_ptr(), 0) };
NonNull::new(cls.cast()).map(|cls| Self { cls })
}
Expand Down Expand Up @@ -255,15 +255,19 @@ impl ClassBuilder {
let types = method_type_encoding(&F::Ret::ENCODING, encs);
let success = Bool::from_raw(unsafe {
ffi::class_addMethod(
self.as_ptr(),
sel.as_ptr() as _,
self.as_mut_ptr(),
sel.as_ptr().cast(),
Some(func.__imp()),
types.as_ptr(),
)
});
assert!(success.as_bool(), "Failed to add method {:?}", sel);
}

fn metaclass_mut(&mut self) -> *mut ffi::objc_class {
unsafe { ffi::object_getClass(self.as_mut_ptr().cast()) as *mut ffi::objc_class }
}

/// Adds a class method with the given name and implementation.
///
/// # Panics
Expand All @@ -290,11 +294,10 @@ impl ClassBuilder {
);

let types = method_type_encoding(&F::Ret::ENCODING, encs);
let metaclass = unsafe { self.cls.as_ref() }.metaclass() as *const _ as *mut _;
let success = Bool::from_raw(unsafe {
ffi::class_addMethod(
metaclass,
sel.as_ptr() as _,
self.metaclass_mut(),
sel.as_ptr().cast(),
Some(func.__imp()),
types.as_ptr(),
)
Expand All @@ -314,7 +317,7 @@ impl ClassBuilder {
let align = log2_align_of::<T>();
let success = Bool::from_raw(unsafe {
ffi::class_addIvar(
self.as_ptr(),
self.as_mut_ptr(),
c_name.as_ptr(),
size,
align,
Expand All @@ -330,7 +333,7 @@ impl ClassBuilder {
///
/// If the protocol wasn't successfully added.
pub fn add_protocol(&mut self, proto: &Protocol) {
let success = unsafe { ffi::class_addProtocol(self.as_ptr(), proto.as_ptr()) };
let success = unsafe { ffi::class_addProtocol(self.as_mut_ptr(), proto.as_ptr()) };
let success = Bool::from_raw(success).as_bool();
assert!(success, "Failed to add protocol {:?}", proto);
}
Expand All @@ -341,15 +344,15 @@ impl ClassBuilder {
/// to the newly registered [`Class`].
pub fn register(self) -> &'static Class {
// Forget self, otherwise the class will be disposed in drop
let cls = ManuallyDrop::new(self).cls;
unsafe { ffi::objc_registerClassPair(cls.as_ptr().cast()) };
unsafe { cls.as_ref() }
let mut this = ManuallyDrop::new(self);
unsafe { ffi::objc_registerClassPair(this.as_mut_ptr()) };
unsafe { this.cls.as_ref() }
}
}

impl Drop for ClassBuilder {
fn drop(&mut self) {
unsafe { ffi::objc_disposeClassPair(self.as_ptr()) }
unsafe { ffi::objc_disposeClassPair(self.as_mut_ptr()) }
}
}

Expand All @@ -369,7 +372,7 @@ unsafe impl Send for ProtocolBuilder {}
unsafe impl Sync for ProtocolBuilder {}

impl ProtocolBuilder {
fn as_ptr(&self) -> *mut ffi::objc_protocol {
fn as_mut_ptr(&mut self) -> *mut ffi::objc_protocol {
self.proto.as_ptr().cast()
}

Expand All @@ -378,8 +381,8 @@ impl ProtocolBuilder {
/// Returns [`None`] if the protocol couldn't be allocated.
pub fn new(name: &str) -> Option<Self> {
let c_name = CString::new(name).unwrap();
let proto = unsafe { ffi::objc_allocateProtocol(c_name.as_ptr()) } as *mut Protocol;
NonNull::new(proto).map(|proto| Self { proto })
let proto = unsafe { ffi::objc_allocateProtocol(c_name.as_ptr()) };
NonNull::new(proto.cast()).map(|proto| Self { proto })
}

fn add_method_description_common<Args, Ret>(
Expand All @@ -403,8 +406,8 @@ impl ProtocolBuilder {
let types = method_type_encoding(&Ret::ENCODING, encs);
unsafe {
ffi::protocol_addMethodDescription(
self.as_ptr(),
sel.as_ptr() as _,
self.as_mut_ptr(),
sel.as_ptr().cast(),
types.as_ptr(),
Bool::new(is_required).as_raw(),
Bool::new(is_instance_method).as_raw(),
Expand Down Expand Up @@ -433,15 +436,15 @@ impl ProtocolBuilder {
/// Adds a requirement on another protocol.
pub fn add_protocol(&mut self, proto: &Protocol) {
unsafe {
ffi::protocol_addProtocol(self.as_ptr(), proto.as_ptr());
ffi::protocol_addProtocol(self.as_mut_ptr(), proto.as_ptr());
}
}

/// Registers the [`ProtocolBuilder`], consuming it and returning a reference
/// to the newly registered [`Protocol`].
pub fn register(self) -> &'static Protocol {
pub fn register(mut self) -> &'static Protocol {
unsafe {
ffi::objc_registerProtocol(self.as_ptr());
ffi::objc_registerProtocol(self.as_mut_ptr());
self.proto.as_ref()
}
}
Expand Down
10 changes: 5 additions & 5 deletions objc2/src/exception.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ unsafe fn try_no_ret<F: FnOnce()>(closure: F) -> Result<(), Option<Id<Object, Sh
let f: extern "C" fn(*mut c_void) = unsafe { mem::transmute(f) };
// Wrap the closure in an Option so it can be taken
let mut closure = Some(closure);
let context = &mut closure as *mut _ as *mut c_void;
let context: *mut Option<F> = &mut closure;
let context = context.cast();

let mut exception = ptr::null_mut();
let success = unsafe { ffi::rust_objc_sys_0_2_try_catch_exception(f, context, &mut exception) };
Expand All @@ -76,7 +77,7 @@ unsafe fn try_no_ret<F: FnOnce()>(closure: F) -> Result<(), Option<Id<Object, Sh
// instance any more, and Rust code is forbidden by requiring a Shared
// Id in `throw` (instead of just a shared reference, which could have
// come from an Owned Id).
Err(unsafe { Id::new(exception as *mut Object) })
Err(unsafe { Id::new(exception.cast()) })
}
}

Expand Down Expand Up @@ -146,8 +147,7 @@ mod tests {
let obj: Id<Object, Shared> = unsafe { Id::new(msg_send![class!(NSObject), new]).unwrap() };

let result = unsafe { catch(|| throw(Some(&obj))) };
let e = result.unwrap_err().unwrap();
// Compare pointers
assert_eq!(&*e as *const Object, &*obj as *const Object);
let exception = result.unwrap_err().unwrap();
assert!(ptr::eq(&*exception, &*obj));
}
}
11 changes: 7 additions & 4 deletions objc2/src/message/apple/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,14 @@ where
A: MessageArguments,
R: Encode,
{
let sup = ffi::objc_super {
receiver: receiver as *mut _,
super_class: superclass as *const Class as *const _,
let superclass: *const Class = superclass;
let mut sup = ffi::objc_super {
receiver: receiver.cast(),
super_class: superclass.cast(),
};
let receiver = &sup as *const ffi::objc_super as *mut Object;
let receiver: *mut ffi::objc_super = &mut sup;
let receiver = receiver.cast();

let msg_send_fn = R::MSG_SEND_SUPER;
unsafe { conditional_try(|| A::__invoke(msg_send_fn, receiver, sel, args)) }
}
11 changes: 6 additions & 5 deletions objc2/src/message/gnustep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ where
return unsafe { mem::zeroed() };
}

let sel_ptr = sel.as_ptr() as *const _;
let msg_send_fn = unsafe { ffi::objc_msg_lookup(receiver as *mut _, sel_ptr) };
let sel_ptr = sel.as_ptr().cast();
let msg_send_fn = unsafe { ffi::objc_msg_lookup(receiver.cast(), sel_ptr) };
let msg_send_fn = msg_send_fn.expect("Null IMP");
unsafe { conditional_try(|| A::__invoke(msg_send_fn, receiver, sel, args)) }
}
Expand All @@ -44,11 +44,12 @@ where
return unsafe { mem::zeroed() };
}

let superclass: *const Class = superclass;
let sup = ffi::objc_super {
receiver: receiver as *mut _,
super_class: superclass as *const Class as *const _,
receiver: receiver.cast(),
super_class: superclass.cast(),
};
let sel_ptr = sel.as_ptr() as *const _;
let sel_ptr = sel.as_ptr().cast();
let msg_send_fn = unsafe { ffi::objc_msg_lookup_super(&sup, sel_ptr) };
let msg_send_fn = msg_send_fn.expect("Null IMP");
unsafe { conditional_try(|| A::__invoke(msg_send_fn, receiver, sel, args)) }
Expand Down
Loading

0 comments on commit b8f9b88

Please sign in to comment.