Some changes to RawFdContainer

[eduardosm]

• Methods are not provided at all on non-unix systems or when allow-unsafe-code is disabled. Trying to create a RawFdContainer when it is not available will cause a compile error instead of panicking at runtime.
• drop does not panic on failure. Instead, a close method is provided that allows the caller to check for errors.
• A try_clone method has been added. It allows to clone a RawFdContainer creating a new FD with dup.

[eduardosm]

* Methods are not provided at all on non-unix systems or when `allow-unsafe-code` is disabled.

Hmm, this is causing issues on windows + libxcb

[psychon]

Basically 👍 for the PR.

Ultimately, it feels to me like that raw fd stuff is something that is not well supported by Rust. That seems like the source of some of the problems we experience.

The old code was generic enough to "support anything". No idea how to easily preserve that. Or, as Travis puts it:

error[E0277]: the trait bound `x11rb::utils::RawFdContainer: std::convert::From<std::fs::File>` is not satisfied
   --> examples/shared_memory.rs:105:10
    |
105 |     conn.shm_attach_fd(shmseg, file, false)?;
    |          ^^^^^^^^^^^^^ the trait `std::convert::From<std::fs::File>` is not implemented for `x11rb::utils::RawFdContainer`
    |
    = note: required because of the requirements on the impl of `std::convert::Into<x11rb::utils::RawFdContainer>` for `std::fs::File`

Implementing conversion for "everything" by hand cannot really work. It would only ever cover stuff that is in std and would break for any new additions to std.

Hmm, this is causing issues on windows + libxcb

That does not seem too bad and can easily be fixed with some feature checks in xcb_ffi:

rror[E0599]: no function or associated item named `into_raw_fd` found for struct `utils::raw_fd_container::RawFdContainer` in the current scope
   --> src\xcb_ffi\mod.rs:220:71
    |
220 |             let mut fds: Vec<_> = fds.into_iter().map(RawFdContainer::into_raw_fd).collect();
    |                                                                       ^^^^^^^^^^^ function or associated item not found in `utils::raw_fd_container::RawFdContainer`
    | 
   ::: src\utils.rs:182:5
    |
182 |     pub struct RawFdContainer(());
    |     ------------------------------ function or associated item `into_raw_fd` not found for this
error[E0599]: no function or associated item named `into_raw_fd` found for struct `utils::raw_fd_container::RawFdContainer` in the current scope
   --> src\xcb_ffi\mod.rs:220:71
    |
220 |             let mut fds: Vec<_> = fds.into_iter().map(RawFdContainer::into_raw_fd).collect();
    |                                                                       ^^^^^^^^^^^ function or associated item not found in `utils::raw_fd_container::RawFdContainer`
    | 
   ::: src\utils.rs:182:5
    |
182 |     pub struct RawFdContainer(());
    |     ------------------------------ function or associated item `into_raw_fd` not found for this
error: aborting due to previous error
For more information about this error, try `rustc --explain E0599`.
error: could not compile `x11rb`.

Methods are not provided at all on non-unix systems or when allow-unsafe-code is disabled. Trying to create a RawFdContainer when it is not available will cause a compile error instead of panicking at runtime.

I think originally I wanted to have the RawFdContainer type only when it would actually work, but then code in src/generated/ related to FD passing failed to compile. Having the type panic at runtime seemed like the easiest way out.

[psychon]

Hmm, this is causing issues on windows + libxcb

You could add an explicit panic when XCBConnection receives an fd and allow-unsafe-code is disabled.

[eduardosm]

Hmm, this is causing issues on windows + libxcb

You could add an explicit panic when XCBConnection receives an fd and allow-unsafe-code is disabled.

In that case, XCBConnection would not be available at all.

I think originally I wanted to have the RawFdContainer type only when it would actually work, but then code in src/generated/ related to FD passing failed to compile. Having the type panic at runtime seemed like the easiest way out.

I was thinking adding something like a FdPassingConnection trait, that would be implemented for connections that support FD passing.

I don't have much time now, so I will continue to work on this later.

[psychon]

I was thinking adding something like a FdPassingConnection trait, that would be implemented for connections that support FD passing.

Hm. Such a change would carry through to all of the generated code (well, at least for those extension that have FD passing stuff). And it would make the return type of x11rb::connect depend on the enabled features (which is not notice and not easily possibly).

Also, I still want to make RustConnection support FD passing, so I am not sure if it makes much sense to spend a lot of time on it.

How about using the nix crate so that nix::unixstd::close and nix::unixstd::dup are accessible without using unsafe? This is basically the route I plan for FD passing on RustConnection. However, that would still not work on Windows.

[psychon]

Could you update/fix the docs? (I guess just removing the last sentence is the enough)

[eduardosm]

Done

[psychon]

This makes nix appear in the public API. I think I would like to avoid that.

I would propose something like this instead:

x11rb/src/rust_connection/stream.rs

Lines 154 to 155 in f792a01

	// Nothing touched errno since sendmsg() failed
	let res = res.map_err(|_| std::io::Error::last_os_error())?;

[eduardosm]

Done

[psychon]

Suggested change

	/// will keep the ownership of is FD.
	/// will keep the ownership of its FD.

[eduardosm]

Fixed

[psychon]

Same as above about nix::Result appearing in the public API

[psychon]

Please copy the doc from the unix version of RawFdContainer (this still talks about allow-unsafe-code)

[eduardosm]

Fixed

[psychon]

Could you move the comment (or something like it) into the argument of unreachable!? (I think unreachable!("foo") should work, does it?)

[eduardosm]

Done