AltF02 / x11-rs

Rust bindings for X11 libraries
https://docs.rs/x11
MIT License
206 stars 66 forks source link

Function to cast event types without copying #38

Closed wolfiestyle closed 8 years ago

wolfiestyle commented 8 years ago

I use this in my code, and I think it would be a great addition to the library:

fn cast_event<T: From<xlib::XEvent>>(ev: &xlib::XEvent) -> &T
{
    unsafe { mem::transmute(ev) }
}

it should be safe because the size of XEvent is >= the size of T (tested elsewhere). Using From<XEvent> does an unnecessary copy and this provides an alternative.

ghost commented 8 years ago

This could be unsafe if another crate implements From<XEvent> for a type not defined in x11-rs. Another way to accomplish the same thing could be to add a function for each type such as:

impl XEvent {
    pub fn as_x_client_message_event (&self) -> &XClientMessageEvent {
        unsafe { mem::transmute(self) }
    }
}

And vice versa:

impl XClientMessageEvent {
    pub fn as_x_event (&self) -> &XEvent {
        unsafe { mem::transmute(self) }
    }
}

A local macro can be used to make these much less verbose.

I recall reading somewhere that it is generally preferred to pass structs by value if they are copyable, but I wonder if the Xlib structures are big enough (I believe XEvent is 128 bytes) for the impact to be significant. Either way, it doesn't hurt to have this option available.

ghost commented 8 years ago

On second thought, this would only work when converting from XEvent to the other types. The reason is that these structs each have different sizes, which would cause undefined behavior after converting the reference to a larger struct and trying to access an element near the end. We could implement AsRef to get a reference to a concrete event type from XEvent, but not the other way around.

wolfiestyle commented 8 years ago

On C this is handled by using an union. What about converting it into a Rust enum?

ghost commented 8 years ago

With the changes in 4a0c484, you should be able to use XEvent like this:

let mut xevent: XEvent = mem::zeroed();

{
    let mut xclient: &mut XClientMessageEvent = xevent.as_mut();
    xclient.type_ = xlib::ClientMessage;
    xclient.message_type = wm_protocols;
    xclient.format = 32;
    xclient.data.set_long(0, wm_delete_window);
}

xlib::XSendEvent(display, window, xlib::False, 0, &mut xevent);
wolfiestyle commented 8 years ago

A pretty nice solution. But XGenericEventCookie was left out.

ghost commented 8 years ago

22dff4d and the changes are published :D

wolfiestyle commented 8 years ago

Thanks!

BTW I think the union RFC was approved some time ago, so we won't need this type of stuff in the future.