remove InetAddr from streamer/src/recvmmsg.rs #558
Conversation
streamer/src/recvmmsg.rs
Outdated
| @@ -44,21 +45,29 @@ pub fn recv_mmsg(socket: &UdpSocket, packets: &mut [Packet]) -> io::Result</*num | |||
|
|
|||
| #[cfg(target_os = "linux")] | |||
| #[allow(deprecated)] | |||
There was a problem hiding this comment.
ah yeah. let me push a new commit to fix this one
| return Some(SocketAddr::V4(SocketAddrV4::new( | ||
| Ipv4Addr::from(addr.sin_addr.s_addr.to_ne_bytes()), | ||
| u16::from_be(addr.sin_port), | ||
| ))); |
There was a problem hiding this comment.
I don't know if these conversions are correct, and it is odd that one uses to_ne_bytes and the other from_be.
Is there any reference for where these come from?
Or can we add some test coverage here?
Similarly for Ipv6Addr below.
There was a problem hiding this comment.
I literally copy from socket2: https://github.com/rust-lang/socket2/blob/65085d9dff270e588c0fbdd7217ec0b392b05ef2/src/sockaddr.rs#L165-L193
There was a problem hiding this comment.
Can you please put that link as a comment in the code?
I also see they have a different code for #[cfg(windows)]. any reason to not include that here?
There was a problem hiding this comment.
Can you please put that link as a comment in the code?
sure! fixed: 0d04bf1
I also see they have a different code for #[cfg(windows)]. any reason to not include that here?
afaik cast_socket_addr has #[cfg(target_os = "linux")] so it will never touch the windows condition
https://github.com/anza-xyz/agave/pull/558/files#diff-2d88fdbf6278e2d0250658ffc2903c9a6701e9b68e3e2303a23691b50bc298e2R46-R47
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #558 +/- ##
=========================================
- Coverage 81.8% 81.8% -0.1%
=========================================
Files 849 849
Lines 229183 229189 +6
=========================================
- Hits 187585 187564 -21
- Misses 41598 41625 +27 |
Problem
#556
Summary of Changes
remove InetAddr from streamer/src/recvmmsg.rs