Skip to content

adds support for AF_IUCV sockets - #668

Open
clayton615 wants to merge 1 commit into
rust-lang:masterfrom
clayton615:master
Open

adds support for AF_IUCV sockets#668
clayton615 wants to merge 1 commit into
rust-lang:masterfrom
clayton615:master

Conversation

@clayton615

Copy link
Copy Markdown

This is my first attempt to add IUCV support. This references issue #667 .

The way I've tried to restrict this to only platforms with IUCV headers (as far as I can tell, this is GNU Linux on x86_64 and s390x) is with the following:

#[cfg(all(
    feature = "iucv",
    any(target_arch = "x86_64", target_arch = "s390x"),
    target_env = "gnu"
))]

Let me know if that can be improved.

I've also added an is_iucv and as_socket_iucv, doing my best to model that off of the existing functions.

I've tested this on x86_64 Linux, and s390x Linux and it's all working as I hoped. I'm hoping that like libc you'll have a suite of tests to make sure I'm not introducing any issues in any platforms I'm not able test.

Scrutiny welcome, always looking to improve.

@clayton615

Copy link
Copy Markdown
Author

Off to a good start. Clippy I can fix (doing so now), but I need some advice for the other two.

For the external types checking, does that mean I'm not allowed to return a libc::sockaddr_iucv? It was the easiest way I could think of to get to the user_id and name fields.

For 3ds, I'm surprised my cfg clause didn't prevent that, unless the runner is just busted?

@Thomasdezeeuw

Copy link
Copy Markdown
Collaborator

For the external types checking, does that mean I'm not allowed to return a libc::sockaddr_iucv? It was the easiest way I could think of to get to the user_id and name fields.

That's correct, we don't want to depend on any external libraries in our API otherwise we can change that dependency version (e.g. going from libc 0.2 to libc 1.0 would be a breaking change, even though the structure would likely not change).

For 3ds, I'm surprised my cfg clause didn't prevent that, unless the runner is just busted?

You can ignore that one, it's a problem with the standard library

@clayton615

Copy link
Copy Markdown
Author

That's correct, we don't want to depend on any external libraries in our API otherwise we can change that dependency version (e.g. going from libc 0.2 to libc 1.0 would be a breaking change, even though the structure would likely not change).

Okay, that makes sense, but I'm less sure how to proceed now. Since there's no SockAddrIucv in the standard library to mirror SockAddrV4 and SockAddrV6 I can't just return one. I see two options:

  1. Add a SockAddrIucv in socket2 and basically just wrap sockaddr_iucv
  2. Instead of using a as_socket_iucv function, I could just add two functions like get_iucv_user and get_iucv_name
    Do you have a preference?

@Thomasdezeeuw

Copy link
Copy Markdown
Collaborator

Okay, that makes sense, but I'm less sure how to proceed now. Since there's no SockAddrIucv in the standard library to mirror SockAddrV4 and SockAddrV6 I can't just return one. I see two options:

  1. Add a SockAddrIucv in socket2 and basically just wrap sockaddr_iucv
  2. Instead of using a as_socket_iucv function, I could just add two functions like get_iucv_user and get_iucv_name
    Do you have a preference?

Have you looked at SockAddrStorage, specifically SockAddrStorage::view_as? It allows you to view a SockAddr (via SockAddr::as_storage) as any storage type. Could you try that to get the sockaddr_iucv address, see if that is ergonomical enough?

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.

2 participants