sys::socket adding sockopt::LingerSec for Apple targets. #2572
Conversation
devnexen
commented
Dec 27, 2024
devnexen
commented
test/sys/test_sockopt.rs
Outdated
| #[cfg(apple_targets)] | ||
| sockopt_impl!( | ||
| LingerSec, | ||
| Both, | ||
| libc::SOL_SOCKET, | ||
| libc::SO_LINGER_SEC, | ||
| libc::linger | ||
| ); |
There was a problem hiding this comment.
Looks like you forgot to remove this:)
There was a problem hiding this comment.
No I did the same than with the above test.
There was a problem hiding this comment.
Emmm, why do we need to define it here?
There was a problem hiding this comment.
I can try to "fix" both tests if you like ? I guess this is because of this ?
There was a problem hiding this comment.
Hmm, this module mod sockopt_impl { is for testing the sockopt_impl! macro, we made it and related stuff public in #2556 so that users can define socket options without contributing to Nix. We should not put the fn test_linger_sec() test here
There was a problem hiding this comment.
Please put the test before that module, and if so, I guess we do not need to re-define the LingerSec socket option
src/sys/socket/sockopt.rs
Outdated
| /// `SO_LINGER_SEC` on apple targets is the genuine equivalent to | ||
| /// other platforms `SO_LINGER`. Indeed, the latter uses ticks. |
There was a problem hiding this comment.
is the genuine equivalent to other platforms
SO_LINGER.
This makes me feel that macOS does not provide SO_LINGER but only SO_LINGER_SEC, can we just say something like:
Same as
SO_LINGETR, but the duration is in seconds rather than kernel ticks.
There was a problem hiding this comment.
This makes me feel that macOS does not provide
SO_LINGERbut onlySO_LINGER_SEC
oh apple does provide it (see my commit message, that s the C header snapshot).
, can we just say something like:
Same as
SO_LINGETR, but the duration is in seconds rather than kernel ticks.
Sure.
devnexen
force-pushed
the
sockopt_so_linger_sec
branch
from
925d413 to
28d4ca9
Compare
test/sys/test_sockopt.rs
Outdated
| sockopt_impl!( | ||
| Linger, | ||
| Both, | ||
| libc::SOL_SOCKET, | ||
| libc::SO_LINGER, | ||
| libc::linger | ||
| ); | ||
| #[test] | ||
| fn test_linger() { | ||
| let fd = socket( | ||
| AddressFamily::Inet, | ||
| SockType::Stream, | ||
| SockFlag::empty(), | ||
| None, | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| let set_linger = libc::linger { | ||
| l_onoff: 1, | ||
| l_linger: 42, | ||
| }; | ||
| setsockopt(&fd, Linger, &set_linger).unwrap(); | ||
|
|
||
| let get_linger = getsockopt(&fd, Linger).unwrap(); | ||
| assert_eq!(get_linger.l_linger, set_linger.l_linger); | ||
| } | ||
|
|
There was a problem hiding this comment.
Other changes look good. Could you please not remove this test from this module, it is for user-defined socket options rather than the one provided by Nix
devnexen
force-pushed
the
sockopt_so_linger_sec
branch
from
47e027a to
4b21447
Compare
