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 UdpSocket::peek method #7068

Merged
merged 4 commits into from
Jan 6, 2025
Merged

Add UdpSocket::peek method #7068

merged 4 commits into from
Jan 6, 2025

Conversation

al8n
Copy link
Contributor

@al8n al8n commented Jan 5, 2025

The mio::net::UdpSocket supports peek already, but tokio::net::UdpSocket is missing this method.

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-net Module: tokio/net labels Jan 5, 2025
@Darksonn
Copy link
Contributor

Darksonn commented Jan 5, 2025

Thanks for the PR!

Currently, this method is based on the mio method; I see the docs are taken from there. However, we generally try to follow the methods in std instead. I notice that std has both peek and peek_from, so we should also have both. Furthermore, to support async usage, we should have both an async fn and a poll_* method.

Based on the above, I think we should have the following methods:

  • peek taking &mut [u8]
  • peek_from taking &mut [u8]
  • poll_peek taking &mut ReadBuf<'_>
  • poll_peek_from taking &mut ReadBuf<'_>

For inspiration, please see the TcpStream::peek and TcpStream::poll_peek methods. The documentation should be similar to them.

@al8n
Copy link
Contributor Author

al8n commented Jan 5, 2025

Thanks for the PR!

Currently, this method is based on the mio method; I see the docs are taken from there. However, we generally try to follow the methods in std instead. I notice that std has both peek and peek_from, so we should also have both. Furthermore, to support async usage, we should have both an async fn and a poll_* method.

Based on the above, I think we should have the following methods:

  • peek taking &mut [u8]
  • peek_from taking &mut [u8]
  • poll_peek taking &mut ReadBuf<'_>
  • poll_peek_from taking &mut ReadBuf<'_>

For inspiration, please see the TcpStream::peek and TcpStream::poll_peek methods. The documentation should be similar to them.

peek_from and poll_peek_from are already there, I will add poll_peek.

The document is copied from the peek_from method. Should I also change the document for peek_from and poll_peek_from?

@Darksonn
Copy link
Contributor

Darksonn commented Jan 6, 2025

Sorry, my first response was inaccurate. What you have now looks very reasonable. Can you add try_peek too?

@al8n
Copy link
Contributor Author

al8n commented Jan 6, 2025

Sorry, my first response was inaccurate. What you have now looks very reasonable. Can you add try_peek too?

Yes, add it.

Copy link
Contributor

@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.

Thanks!

@Darksonn Darksonn merged commit acd6627 into tokio-rs:master Jan 6, 2025
83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-net Module: tokio/net
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants