Special case assume_init for Bool on Windows#655
Conversation
| } | ||
| } | ||
|
|
||
| impl<T: Copy> AssumeInit for T {} |
There was a problem hiding this comment.
Note that this fakes specialisation by not implementing Copy for the type (i.e. Bool) we want to special case. This is fine for our usage since we only actually need to move values and Bool is private.
There was a problem hiding this comment.
What other types is this required by? Could we just explicitly implement it for the 3 or 4 other types?
There was a problem hiding this comment.
It would currently need to be implemented for i32, u32, IN_ADDR, linger and WSAPROTOCOL_INFOW. Which seems fine. My only reluctance was that more things could be added in the future.
There was a problem hiding this comment.
I would consider doing that and renaming this to GetsockoptOutput or similar.
Thomasdezeeuw
left a comment
Thomasdezeeuw
left a comment
There was a problem hiding this comment.
This seems like a reasonable solution. Though I'm not super happy that we need to add this much code.
| /// | ||
| /// # Safety | ||
| /// | ||
| /// `size` must be either 1 or 4 and that many bytes must have been initialized. |
There was a problem hiding this comment.
Nit: we don't need this documentation, the trait itself already explains it and the implementation does the rest.
| debug_assert_eq!(optlen as usize, mem::size_of::<T>()); | ||
| // Safety: `getsockopt` initialised `optval` for us. | ||
| optval.assume_init() | ||
| T::assume_init(optval, optlen as usize) |
There was a problem hiding this comment.
Nit: do we want change the trait to accept an c_int? That way we don't need the cast here.
There was a problem hiding this comment.
Sure, that does make sense to me.
|
GitHub Actions was stuck (what else is new), so I force pushed the commit to get it to start. |
|
Thanks @ChrisDenton |
3 participants
On Windows many socket options are documented as having the value "
DWORD(boolean)". ADWORDis 4 bytes but when writing a boolean, often only one byte is actually written. While this is mostly consistent, it has been observed that usinggetsockoptwith theIPV6_V6ONLYoption can sometimes write 1 byte but other time write 4 bytes. So this PR allows us to accept either 1 or 4 bytes when the type of the option isBool.Fixes #564