Skip to content

Commit

Permalink
Fix double freeing of strings
Browse files Browse the repository at this point in the history
  • Loading branch information
madsmtm committed Sep 5, 2023
1 parent db7b3d8 commit ba252ff
Show file tree
Hide file tree
Showing 8 changed files with 12 additions and 33 deletions.
2 changes: 1 addition & 1 deletion src/appkit/toolbar/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ extern "C" fn selectable_item_identifiers<T: ToolbarDelegate>(this: &Object, _:
/// Objective-C runtime needs.
extern "C" fn item_for_identifier<T: ToolbarDelegate>(this: &Object, _: Sel, _: id, identifier: id, _: Bool) -> id {
let toolbar = load::<T>(this, TOOLBAR_PTR);
let identifier = NSString::from_retained(identifier);
let identifier = NSString::retain(identifier);

let item = toolbar.item_for(identifier.to_str());
unsafe { msg_send![&*item.objc, self] }
Expand Down
6 changes: 0 additions & 6 deletions src/foundation/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,6 @@ impl NSArray {
NSArray(unsafe { Id::retain(array).unwrap() })
}

/// In some cases, we're vended an `NSArray` by the system, and it's ideal to not retain that.
/// This handles that edge case.
pub fn from_retained(array: id) -> Self {
NSArray(unsafe { Id::new(array).unwrap() })
}

/// Returns the `count` (`len()` equivalent) for the backing `NSArray`.
pub fn count(&self) -> usize {
unsafe { msg_send![&*self.0, count] }
Expand Down
6 changes: 0 additions & 6 deletions src/foundation/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,6 @@ impl NSData {
NSData(unsafe { Id::retain(data).unwrap() })
}

/// If we're vended an NSData from a method (e.g, a push notification token) we might want to
/// wrap it while we figure out what to do with it. This does that.
pub fn from_retained(data: id) -> Self {
NSData(unsafe { Id::new(data).unwrap() })
}

/// A helper method for determining if a given `NSObject` is an `NSData`.
pub fn is(obj: id) -> bool {
let result: BOOL = unsafe { msg_send![obj, isKindOfClass: class!(NSData)] };
Expand Down
7 changes: 3 additions & 4 deletions src/foundation/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,9 @@ impl<'a> NSString<'a> {
}
}

/// In some cases, we want to wrap a system-provided NSString without retaining it.
pub fn from_retained(object: id) -> Self {
NSString {
objc: unsafe { Id::new(object).unwrap() },
pub fn from_id(objc: Id<Object, Owned>) -> Self {
Self {
objc,
phantom: PhantomData
}
}
Expand Down
8 changes: 0 additions & 8 deletions src/foundation/urls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,6 @@ impl<'a> NSURL<'a> {
}
}

/// In some cases, we want to wrap a system-provided NSURL without retaining it.
pub fn from_retained(object: id) -> Self {
NSURL {
objc: unsafe { Id::new(object).unwrap() },
phantom: PhantomData
}
}

/// Creates and returns a URL object by calling through to `[NSURL URLWithString]`.
pub fn with_str(url: &str) -> Self {
let url = NSString::new(url);
Expand Down
4 changes: 2 additions & 2 deletions src/text/attributed_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ impl AttributedString {

impl fmt::Display for AttributedString {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let string = NSString::from_retained(unsafe { msg_send![&*self.0, string] });
let string = NSString::from_id(unsafe { msg_send_id![&*self.0, string] });

write!(f, "{}", string.to_str())
}
}

impl fmt::Debug for AttributedString {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let string = NSString::from_retained(unsafe { msg_send![&*self.0, string] });
let string = NSString::from_id(unsafe { msg_send_id![&*self.0, string] });

f.debug_struct("AttributedString").field("text", &string.to_str()).finish()
}
Expand Down
2 changes: 1 addition & 1 deletion src/uikit/scene/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ impl SceneSession {
}

pub fn role(&self) -> SessionRole {
NSString::from_retained(unsafe { msg_send![&*self.0, role] }).into()
NSString::from_id(unsafe { msg_send_id![&*self.0, role] }).into()
}
}
10 changes: 5 additions & 5 deletions src/webview/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use block::Block;

use objc::declare::ClassDecl;
use objc::runtime::{Bool, Class, Object, Sel};
use objc::{class, msg_send, sel};
use objc::{class, msg_send, msg_send_id, sel};

use crate::foundation::{id, load_or_register_class, nil, NSArray, NSInteger, NSString};
use crate::webview::actions::{NavigationAction, NavigationResponse};
Expand Down Expand Up @@ -48,8 +48,8 @@ extern "C" fn on_message<T: WebViewDelegate>(this: &Object, _: Sel, _: id, scrip
let delegate = load::<T>(this, WEBVIEW_DELEGATE_PTR);

unsafe {
let name = NSString::from_retained(msg_send![script_message, name]);
let body = NSString::retain(msg_send![script_message, body]);
let name = NSString::from_id(msg_send_id![script_message, name]);
let body = NSString::from_id(msg_send_id![script_message, body]);
delegate.on_message(name.to_str(), body.to_str());
}
}
Expand All @@ -62,7 +62,7 @@ extern "C" fn start_url_scheme_task<T: WebViewDelegate>(this: &Object, _: Sel, _
let request: id = msg_send![task, request];
let url: id = msg_send![request, URL];

let uri = NSString::from_retained(msg_send![url, absoluteString]);
let uri = NSString::from_id(msg_send_id![url, absoluteString]);
let uri_str = uri.to_str();

if let Some(content) = delegate.on_custom_protocol_request(uri_str) {
Expand Down Expand Up @@ -152,7 +152,7 @@ extern "C" fn handle_download<T: WebViewDelegate>(this: &Object, _: Sel, downloa
let delegate = load::<T>(this, WEBVIEW_DELEGATE_PTR);

let handler = handler as *const Block<(objc::runtime::Bool, id), ()>;
let filename = NSString::from_retained(suggested_filename);
let filename = NSString::retain(suggested_filename);

delegate.run_save_panel(filename.to_str(), move |can_overwrite, path| unsafe {
if path.is_none() {
Expand Down

0 comments on commit ba252ff

Please sign in to comment.