Skip to content

Is it really safe for QObjectExt::set_parent to accept an immutable reference? #1294

Open
@jnbooth

Description

@jnbooth

Currently, QObjectExt::set_parent accepts a parent: &QObject argument, which it then converts to a mutable pointer:

fn set_parent(self: Pin<&mut Self>, parent: &Self) {
unsafe {
cast_pin(self)
.set_parent(cast(parent) as *const QObjectExternal as *mut QObjectExternal);
}
}

Is this really safe? QObject::setParent() requires a mutable pointer on Qt's side, and its implementation mutates the parent by appending the object to its list of children. It also triggers the ChildAdded event on the parent, which may perform arbitrary code in its event handler.

I would have expected something more like this:

fn set_parent<T>(self: Pin<&mut Self>, parent: Option<Pin<&mut T>>)
where
    T: Upcast<QObject>,
{
    unsafe {
        let parent = match parent {
            Some(parent) => ptr::from_mut(parent.upcast_pin().get_unchecked_mut()).cast(),
            None => ptr::null_mut(),
        };
        cast_pin(self).set_parent(parent);
    }
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    ⚡ SafetyIssues related to Safety - considered critical, can justify a semver-breaking change.🪲 bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions