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

Add Socket::{send,recv}msg and MsgHdr(Mut) #447

Merged
merged 10 commits into from
Jun 8, 2023

Conversation

Thomasdezeeuw
Copy link
Collaborator

This adds wrappers around sendmsg(2) and recvmsg(2).

This wraps msghdr on Unix and WSAMSG on Windows to be used in
send/recvmsg calls.
Wrapper around sendmsg(2).
@Thomasdezeeuw
Copy link
Collaborator Author

I think this is unsound when using the non-mutable version in combination with recvmsg, e.g. recvmsg(fd, &MsgHdr::new().with_addr(&addr)) will write to addr even though we only supplied a read-only reference, which is UB.

I though about solving this with a generic type in MsgHdr<Mutable|ReadOnly> where enum {Mutable,ReadOnly}. Then we can require mutable reference when using MsgHdr<Mutable>.

@yuguorui
Copy link

yuguorui commented Jun 3, 2023

I though about solving this with a generic type in MsgHdr<Mutable|ReadOnly> where enum {Mutable,ReadOnly}. Then we can require mutable reference when using MsgHdr.

Maybe something like this? In this case, we would be leak some types to the users, such as MsgHdrMutableTrait.

Or we just copy and paste to create struct MsgHdr and MsgHdrMut 😄?

pub trait MsgHdrMutableTrait {
     fn mutable(&self);
}

pub struct Mutable {}
impl MsgHdrMutableTrait for Mutable {
    fn mutable(&self) {}
}

pub struct MsgHdr<'addr, 'bufs, 'control, T>
{
    inner: sys::msghdr,
    #[allow(clippy::type_complexity)]
    _lifetimes: PhantomData<(&'addr SockAddr, &'bufs [&'bufs [u8]], &'control [u8])>,
    _mutable: PhantomData<T>,
}

impl<'addr, 'bufs, 'control, T> MsgHdr<'addr, 'bufs, 'control, T> 
where T: MsgHdrMutableTrait
{
    pub fn with_buffers_mut(mut self, bufs: &'bufs mut [IoSlice<'bufs>]) -> Self {
        let ptr = bufs.as_ptr().cast_mut().cast();
        sys::set_msghdr_iov(&mut self.inner, ptr, bufs.len());
        self
    }
}

impl<'addr, 'bufs, 'control, T> MsgHdr<'addr, 'bufs, 'control, T> 
{
    pub fn new() -> MsgHdr<'addr, 'bufs, 'control, T> {
        MsgHdr {
            inner: unsafe { mem::zeroed() },
            _lifetimes: PhantomData,
            _mutable: PhantomData,
        }
    }

    pub fn with_buffers(mut self, bufs: &'bufs [IoSlice<'bufs>]) -> Self {
        let ptr = bufs.as_ptr().cast_mut().cast();
        sys::set_msghdr_iov(&mut self.inner, ptr, bufs.len());
        self
    }
}

fn main() {
    let msghdr = MsgHdr::<Mutable>::new();
    // blah blah
}

Instead we'll create a separate MsgHdrMut for recvmsg, similar to
IoSlice and IoSliceMut.
@Thomasdezeeuw
Copy link
Collaborator Author

I've just added MsgHdrMut following IoSlice and IoSliceMut in std::io. The last commit adds Socket::recvmsg, but it's currently broken on Windows pending microsoft/windows-rs#2530.

@yuguorui
Copy link

yuguorui commented Jun 3, 2023

The support for WSARecvMsg may not be a straightforward task, this API in Windows is very quirky...

WSARecvMsg function must be obtained at run time by making a call to the [WSAIoctl](https://msdn.microsoft.com/en-
us/library/ms741621(v=vs.85)) function with the SIO_GET_EXTENSION_FUNCTION_POINTER opcode specified.

By the way, thank you very much for your efforts! If there is any work that I could help, please let me know directly.

WSARecvMsg doc

@Thomasdezeeuw
Copy link
Collaborator Author

The support for WSARecvMsg may not be a straightforward task, this API in Windows is very quirky...

WSARecvMsg function must be obtained at run time by making a call to the [WSAIoctl](https://msdn.microsoft.com/en-
us/library/ms741621(v=vs.85)) function with the SIO_GET_EXTENSION_FUNCTION_POINTER opcode specified.

By the way, thank you very much for your efforts! If there is any work that I could help, please let me know directly.

WSARecvMsg doc

Yeah, I don't think WSARecvMsg is the way to go. But the WSARecv function doesn't accept a WSAMSG.Control field. Do you know what the recvmsg(2) equivalent on Windows is?

pointer::cast_mut was stablised in 1.65, so it's too new.
@Thomasdezeeuw
Copy link
Collaborator Author

After a couple of Windows exported weighted in on WSARecvMsg (in microsoft/windows-rs#2530) it seems that it is indeed the right function to call. However you need use WSAIoctl to get a function pointer to it, and since Socket doesn't carry any state beyond the handle this is hard to do.

For now I've decided to drop Windows support for Socket::recvmsg and once someone needs it we can look at it again.

@Thomasdezeeuw Thomasdezeeuw marked this pull request as ready for review June 7, 2023 11:23
@Thomasdezeeuw Thomasdezeeuw changed the title Add Socket::{send,recv}msg and MsgHdr Add Socket::{send,recv}msg and MsgHdr(Mut) Jun 7, 2023
Copy link
Collaborator

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Clever, but I agree that this is correct.

Unfortunate that you need these #[cfg(not(target_os = "redox"))] all over the place. Perhaps some of it could be moved to a file to avoid that? I'll let you decide what you prefer here.

@Thomasdezeeuw
Copy link
Collaborator Author

Clever, but I agree that this is correct.

Unfortunate that you need these #[cfg(not(target_os = "redox"))] all over the place. Perhaps some of it could be moved to a file to avoid that? I'll let you decide what you prefer here.

Yes, I agree. I can kind want to cleanup the lib.rs file a bit as well as the way we export types for the sake of documentation as currently it's just a list of structs without a clean indicator that Socket is really the main type, the other just helper types.

But I rather not delay this pr for that.

@Thomasdezeeuw Thomasdezeeuw merged commit 54e7215 into rust-lang:master Jun 8, 2023
37 checks passed
@Thomasdezeeuw Thomasdezeeuw deleted the msghdr branch June 8, 2023 07:30
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

3 participants