Skip to content
This repository has been archived by the owner on May 17, 2018. It is now read-only.

added UnixSeqpacket and UnixSeqpacketListener. #25

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rrichardson
Copy link

plus minor refactoring to consolidate into Inner.

This addresses issue #23

This would be much nicer with generic impl specialization :)

UnixStreamListener and UnixSeqpacketListener are 99% alike. In fact, in their implementations they only differ in 3 words, unfortunately, two of those are in return types, so there wasn't a clean way to factor them out, aside from making some sort of UnixSocket trait. So we're stuck with it unless we want to get crazy.

I moved send and recv into Inner which cleaned up some code in UnixStream and UnixDatagram while reducing the amount to duplicate into UnixSeqpacket.

Added a test for basic seqpacket functionality. All of the rest of the tests still pass.

@rrichardson
Copy link
Author

Depends upon rust-lang/libc#300

@@ -536,13 +717,13 @@ impl IntoRawFd for UnixStream {
/// // close the listener socket
/// drop(listener);
/// ```
pub struct UnixListener {
pub struct UnixStreamListener {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should probably leave this as UnixListener. It's a bit of an unfortunate naming setup, but that's what the standard library setup has, and it's slated for stabilization.

Copy link
Author

Choose a reason for hiding this comment

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

:(
o.k.

On Fri, May 27, 2016, 12:42 AM Steven Fackler notifications@github.com
wrote:

In src/lib.rs
#25 (comment)
:

@@ -536,13 +717,13 @@ impl IntoRawFd for UnixStream {
/// // close the listener socket
/// drop(listener);
/// ```
-pub struct UnixListener {
+pub struct UnixStreamListener {

I think we should probably leave this as UnixListener. It's a bit of an
unfortunate naming setup, but that's what the standard library setup has,
and it's slated for stabilization.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/rust-lang-nursery/unix-socket/pull/25/files/fcb59380b36d9182b7b83c1db74ef0c9f345674c#r64857039,
or mute the thread
https://github.com/notifications/unsubscribe/AAHlC24C1s3x9VvOxC7H9USyEkFBwTLXks5qFnZOgaJpZM4IoKe0
.

@sfackler
Copy link
Member

Yay!

@rrichardson
Copy link
Author

K. It's now UnixListener. Now it just needs the libc branch merged.

@sfackler
Copy link
Member

sfackler commented Jun 2, 2016

Is this still blocked on libc?

@rrichardson
Copy link
Author

Yes. The PR for SEQPACKET was merged but a new version has not been pushed
into crates.io.

On Thu, Jun 2, 2016 at 1:54 AM Steven Fackler notifications@github.com
wrote:

Is this still blocked on libc?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#25 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAHlC2xX5n5NQF0gZR0E3mLpWwxO3wqzks5qHnADgaJpZM4IoKe0
.

@sfackler
Copy link
Member

sfackler commented Jun 2, 2016

(ping @alexcrichton)

@alexcrichton
Copy link
Member

"Seqpacket" here seems kinda long but beyond that seems reasonable to me!

@sfackler
Copy link
Member

A new libc release landed, so you should be able to bump the version to 0.2.12 and get things building.

@rrichardson
Copy link
Author

There is another conflict: libc depends on tempdir which depends on rand which depends on libc 0.2.11.
Should we bump rand up to 0.2.12 as well then?

@sfackler
Copy link
Member

Unless it depends on =0.2.11, those should work just fine together 0.2.11 means >= 0.2.11, < 0.3.

@rrichardson
Copy link
Author

rrichardson commented Jun 10, 2016

I'm trying to sort this out. Rand states that it depends on 0.2, but I'm getting:

error: failed to select a version for `libc` (required by `rand`):
all possible versions conflict with previously selected versions of `libc`
  version 0.2.12 in use by libc v0.2.12
  possible versions to select: 0.2.11

oddly, if I specify I depend on libc = "0.2" instead of "0.2.12" it downloads and compiles 0.2.12.

specifying 0.2.12 states that only 0.2.11 is available.

@sfackler
Copy link
Member

Try a cargo update or deleting Cargo.toml.

@sfackler
Copy link
Member

Looks like this needs a rebase and then it should be ready to go!

@dkl
Copy link

dkl commented Apr 9, 2018

Hi, I'd like to help out with finishing this. @rrichardson, did you still plan to work on it, or is it ok if I rebase and re-submit the pull request?

Or is it better to directly add to libstd now (seeing #27)?

@rrichardson
Copy link
Author

Wow, I totally forgot about this. I guess rebase and give it a go.
If it makes sense to the libstd maintainers, I'd be happy to help make a PR for that.

@dkl
Copy link

dkl commented Apr 29, 2018

Hey again, I tried writing an RFC for it (since it's an addition to std): https://github.com/dkl/rust-rfcs/blob/unixsocket-seqpacket/text/0000-unix-socket-seqpacket.md
Does that seem reasonable? Any suggestions before submitting?

@sfackler
Copy link
Member

The API description in the RFC seems reasonable to me, but I don't think a full RFC is required for an addition like this - a PR direct to add the types (as unstable) should be fine!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants