Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented all std::os::fd traits for mqueue::MqdT. #2097

Merged
merged 10 commits into from
Aug 19, 2023

Conversation

coderBlitz
Copy link
Contributor

Since linux allows polling message queues 1, these changes enable that use case. Targeted to linux, since it was the only one I was able to confirm 2.

Footnotes

  1. mq_overview(7)

  2. https://elixir.bootlin.com/linux/latest/source/ipc/mqueue.c#L908

@coderBlitz coderBlitz marked this pull request as ready for review August 14, 2023 23:04
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only for Linux? Are you sure these traits shouldn't apply more widely?

@coderBlitz
Copy link
Contributor Author

I don't have any reason to artificially limit it, as I mentioned originally it was based on my working knowledge of the mqueue POSIX API and the linux implementation of it. Per the POSIX API (see [1] above), there are no guarantees that mqd_t is a file descriptor, or even an identically-sized integer; the linux implementation guarantees a valid file descriptor.

At its broadest, the API could be enabled for all unix per the Rust standard lib (see below for exceptions):

// unix
#[cfg(not(all(
    doc,
    any(
        all(target_arch = "wasm32", not(target_os = "wasi")),
        all(target_vendor = "fortanix", target_env = "sgx")
    )
)))]
#[cfg(target_os = "hermit")]
#[path = "hermit/mod.rs"]
pub mod unix;
#[cfg(not(all(
    doc,
    any(
        all(target_arch = "wasm32", not(target_os = "wasi")),
        all(target_vendor = "fortanix", target_env = "sgx")
    )
)))]
#[cfg(all(not(target_os = "hermit"), any(unix, doc)))]
pub mod unix;

However, there appear to be 2 libc families which would not satisfy the type guarantees1: freebsdlike and solarish. The rest use the c_int type, so those should all theoretically compile without issue. In my semi-cursory searches, all of the c_int types seem to alias to i32, which matches the RawFd alias, so everything points to those compiling fine.

Footnotes

  1. https://github.com/search?q=repo%3Arust-lang%2Flibc+mqd_t&type=code

@coderBlitz coderBlitz marked this pull request as draft August 19, 2023 15:24
@asomers
Copy link
Member

asomers commented Aug 19, 2023

Man pages suggest that select and poll are supported for message queues on NetBSD and DragonflyBSD. I'm very surprised to see that libc doesn't even define mq_open on Android, so we can't do it there. On FreeBSD select and poll are supported but through a different mechanism: one must first use mq_getfd_np to get the file descriptor. So I think you should enable this for Linux, NetBSD, and DragonflyBSD.

@coderBlitz
Copy link
Contributor Author

coderBlitz commented Aug 19, 2023

I'm slightly concerned the dragonfly build succeeded, given mqd_t in the freebsdlike files is explicitly a pointer. @asomers Thoughts?

@asomers
Copy link
Member

asomers commented Aug 19, 2023

I'm slightly concerned the dragonfly build succeeded, given mqd_t in the freebsdlike files is explicitly a pointer. @asomers Thoughts?

I think you read that wrong. The mqd_t type isn't defined in freebsdlike/mod.rs. It's defined separately in freebsdlike/freebsd/mod.rs and freebsdlike/dragonfly/mod.rs.

src/mqueue.rs Outdated Show resolved Hide resolved
@coderBlitz coderBlitz marked this pull request as ready for review August 19, 2023 17:43
@asomers asomers added this pull request to the merge queue Aug 19, 2023
Merged via the queue into nix-rust:master with commit c93fe51 Aug 19, 2023
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants