AltF02 / x11-rs

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

Creating CStrings required to use xlib::XCreateIC #59

Closed Ryan1729 closed 7 years ago

Ryan1729 commented 7 years ago

The fact that as far as I can tell, something like the below is required in order to call xlib::XCreateIC and have it work correctly is inconvenient, and took some time for me to figure out.

 xlib::XCreateIC(xw.xim,
                             CString::new(xlib::XNInputStyle).unwrap().as_ptr(),
                             xlib::XIMPreeditNothing | xlib::XIMStatusNothing,
                             CString::new(xlib::XNClientWindow).unwrap().as_ptr(),
                             xw.win,
                             CString::new(xlib::XNFocusWindow).unwrap().as_ptr(),
                             xw.win,
                             0 as *mut libc::c_void);

But after doing a bit of research, I couldn't find a nice way to do const CStrings so I can understand why it is this way, in retrospect.

ghost commented 7 years ago

This was an oversight on my part when adding string constants without a '\0' at the end. We shouldn't have to create a CString, allocating memory on the heap, just to use these constants. The best fix I can currently think of that doesn't introduce breaking changes would be something like:

pub const XNInputStyle_0: &'static [u8] = b"inputStyle\0";

xlib::XCreateIC(xw.xim, xlib::XNInputStyle_0.as_ptr() as *const _, ...);

Version 3 (waiting on untagged unions in stable Rust) will get this right the first time.

Ryan1729 commented 7 years ago

Fair enough. I wasn't worried about the heap allocations since the code I'm porting calls this function once at startup. But for a different application it could definitely be a problem.

That solution seems reasonable to me, given the constraints.

I see the docs are auto-generated. Given that these extra constants differ from the original set, it seems like a good idea to add an example like the one you gave to the docs. I don't know how easy the current generation code makes that though.

On Mar 8, 2017, at 9:56 AM, Daggerbot notifications@github.com wrote:

This was an oversight on my part when adding string constants without a '\0' at the end. We shouldn't have to create a CString, allocating memory on the heap, just to use these constants. The best fix I can currently think of that doesn't introduce breaking changes would be something like:

pub const XNInputStyle_0: &'static [u8] = b"inputStyle\0";

xlib::XCreateIC(xw.xim, xlib::XNInputStyle_0.asptr() as *const , ...); — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

ghost commented 7 years ago

Sorry I've taken so long to respond :/ I'm back in school full time (no longer compsci) in addition to working full time, so I haven't had any time for my coding projects. But the semester is almost over so I'll have some time soon.

ghost commented 7 years ago

Added in 4340a85.