Skip to content

Commit 847d1f4

Browse files
authored
Merge pull request #20 from godot-rust/bugfix/crash-optional-init
Fix null-check against uninitialized pointer in `Gd::from_sys_init_opt()`
2 parents 4287594 + 4924b44 commit 847d1f4

File tree

1 file changed

+10
-20
lines changed

1 file changed

+10
-20
lines changed

godot-core/src/obj/gd.rs

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -469,33 +469,23 @@ impl<T: GodotClass> Gd<T> {
469469
pub unsafe fn from_sys_init_opt(init_fn: impl FnOnce(sys::GDNativeTypePtr)) -> Option<Self> {
470470
// Note: see _call_native_mb_ret_obj() in godot-cpp, which does things quite different (e.g. querying the instance binding).
471471

472-
// Much elegant
473-
let mut is_null = false;
474-
let outer_fn = |ptr| {
475-
init_fn(ptr);
476-
477-
// ptr has type TypePtr = OpaqueObject* = Object** (in other words, the opaque encodes an Object*)
478-
// we don't need to know if Object** is null, but if Object* (modified through Object**) is null.
479-
let opaque_ptr = ptr as *const OpaqueObject;
480-
if is_zeroed(*opaque_ptr) {
481-
is_null = true;
482-
}
483-
};
472+
// return_ptr has type GDNativeTypePtr = GDNativeObjectPtr* = OpaqueObject* = Object**
473+
// (in other words, the type-ptr contains the _address_ of an object-ptr).
474+
let mut object_ptr: sys::GDNativeObjectPtr = ptr::null_mut();
475+
let return_ptr: *mut sys::GDNativeObjectPtr = ptr::addr_of_mut!(object_ptr);
476+
477+
init_fn(return_ptr as sys::GDNativeTypePtr);
484478

485-
// TODO forget, ready etc
486-
let gd = Self::from_sys_init(outer_fn);
487-
if is_null {
479+
// We don't need to know if Object** is null, but if Object* is null; return_ptr has the address of a local (never null).
480+
if object_ptr.is_null() {
488481
None
489482
} else {
490-
Some(gd)
483+
let obj = Gd::from_obj_sys(object_ptr); // equivalent to Gd::from_sys(return_ptr)
484+
Some(obj)
491485
}
492486
}
493487
}
494488

495-
fn is_zeroed(opaque: OpaqueObject) -> bool {
496-
unsafe { std::mem::transmute::<OpaqueObject, u64>(opaque) == 0 }
497-
}
498-
499489
/// Destructor with semantics depending on memory strategy.
500490
///
501491
/// * If this `Gd` smart pointer holds a reference-counted type, this will decrement the reference counter.

0 commit comments

Comments
 (0)