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 peek and peek_from to UnixStream and UnixDatagram #73761

Merged
merged 1 commit into from
Sep 11, 2020
Merged

Add peek and peek_from to UnixStream and UnixDatagram #73761

merged 1 commit into from
Sep 11, 2020

Conversation

rijenkii
Copy link
Contributor

This is my first PR, so I'm sure I've done some things wrong.

This PR:

  • adds peek function to UnixStream;
  • adds peek and peek_from to UnixDatagram;
  • moves UnixDatagram::recv_from implementation to a private function recv_from_flags, as peek_from uses the same code, just with different flags.

I've taken the documentation from TcpStream and UdpStream, so it may or may not make sense (I'm bad with english words).
Also, I'm not sure what I should write in the unstable attribute, so I've made up the name and set the issue to "none".

Closes #68565.

@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 26, 2020
@the8472
Copy link
Member

the8472 commented Jun 26, 2020

See also #69864

@Muirrum Muirrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 17, 2020
@bors
Copy link
Contributor

bors commented Jul 28, 2020

☔ The latest upstream changes (presumably #73265) made this pull request unmergeable. Please resolve the merge conflicts.

@Muirrum Muirrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2020
@joshtriplett
Copy link
Member

This seems reasonable to me. It needs to be rebased (rather than back-merging from the latest rust-lang/rust), but otherwise, I think it'd be reasonable to merge this.

@rijenkii
Copy link
Contributor Author

Done, but what about the issue = "none"? And should I add #[feature(unix_socket_peek)] anywhere else?

@bors
Copy link
Contributor

bors commented Aug 31, 2020

☔ The latest upstream changes (presumably #75979) made this pull request unmergeable. Please resolve the merge conflicts.

@daniel5151
Copy link

Just adding my vote of interest to see this get merged!

I've had to roll my own extension trait for UnixStream that adds peek, and while it works fine, it would be nice if I could remove that bit of unsafe code from my project.

@joshtriplett
Copy link
Member

This looks great to me. Could someone on @rust-lang/libs review and merge, please?

@Mark-Simulacrum
Copy link
Member

It looks like this needs a rebase, but I'll r? @KodrAus for libs team unstable API approval. Implementation looks good to me too (r=me or Josh on that).

Copy link
Contributor

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

PR is 80% tests and docs? Looks good to me!

It looks like we've got a merge conflict to work through so once that's sorted I'll push it through.

@rijenkii
Copy link
Contributor Author

I'll fix the conflicts in the next couple of days, a little bit busy rn

@Dylan-DPC-zz Dylan-DPC-zz added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 11, 2020
@rijenkii
Copy link
Contributor Author

Done, I think
Rebases are hard

@Dylan-DPC-zz
Copy link

bors r=KodrAus

@Dylan-DPC-zz
Copy link

gah again ..

@bors r=KodrAus

@bors
Copy link
Contributor

bors commented Sep 11, 2020

📌 Commit 64b8fd7 has been approved by KodrAus

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 11, 2020
@bors
Copy link
Contributor

bors commented Sep 11, 2020

⌛ Testing commit 64b8fd7 with merge 9911160...

@bors
Copy link
Contributor

bors commented Sep 11, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: KodrAus
Pushing 9911160 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 11, 2020
@bors bors merged commit 9911160 into rust-lang:master Sep 11, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 11, 2020
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Please open a tracking issue for the new unstable methods and send a PR updating these issue = "..." attributes with the tracking issue number.

@rijenkii
Copy link
Contributor Author

Tracking issue opened, PR sent

RalfJung added a commit to RalfJung/rust that referenced this pull request Sep 20, 2020
Add tracking issue for feature(unix_socket_peek)

Feature was added in rust-lang#73761
RalfJung added a commit to RalfJung/rust that referenced this pull request Sep 20, 2020
Add tracking issue for feature(unix_socket_peek)

Feature was added in rust-lang#73761
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

peek methods for UnixDatagram and UnixStream?