-
-
Notifications
You must be signed in to change notification settings - Fork 531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
netsocket: socket operations return error codes via return values #4092
Merged
past-due
merged 8 commits into
Warzone2100:master
from
ManManson:netsocket_api_expected_refactoring
Oct 19, 2024
Merged
netsocket: socket operations return error codes via return values #4092
past-due
merged 8 commits into
Warzone2100:master
from
ManManson:netsocket_api_expected_refactoring
Oct 19, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ketSet()` This instead should always throw `std::bad_alloc`. Signed-off-by: Pavel Solodovnikov <[email protected]>
`operator new` will throw a `std::bad_alloc` exception instead of returning `nullptr`, so there's no need to check for `nullptr` after trying to allocate a new `Socket` instance. Signed-off-by: Pavel Solodovnikov <[email protected]>
Signed-off-by: Pavel Solodovnikov <[email protected]>
…slation of network errors Signed-off-by: Pavel Solodovnikov <[email protected]>
Signed-off-by: Pavel Solodovnikov <[email protected]>
Signed-off-by: Pavel Solodovnikov <[email protected]>
…, std::error_code>` Plus, add the necessary dependency for the `netplay` library in order for it to find `tl::expected` header files. Signed-off-by: Pavel Solodovnikov <[email protected]>
Switch to returning socket error codes explicitly via return values instead of using legacy functions `getSockErr()`, `setSockErr`, `strSockError()` to get extended error context. To facilitate that, use `tl::expected<T, std::error_code>` as the return value type for all main socket operations: read, write, open, listen. Moving to this new approach has numerous benefits over the old one: 1. Instead of using POSIX constants directly, we wrap them into `std::error_code` instances, which allows to attach custom error categories to them. This can be used to customize error messages and error codes mapping to platform-independent `std::error_conditions`. 2. Calling separate `get/setSockErr()` functions is error-prone: one can easily forget to check the error condition from `getSockErr()` and the value will be overwritten by the next socket function without the ability to recover the former error. Conversely, one can forget to call `setSockErr()` to set the proper error code for the caller to check upon. 3. As mentioned above, `std::error_code:s` can be implicitly mapped to platform-independent `std::error_conditions`, allowing for this code to compile successfuly: if (errCode == std::errc::connection_reset) { ... } This allows for very convenient and portable error checking code, which completely hides implementation details of how a particular error code is implemented (but, if one really needs to, they still can extract the platform-dependent error code value to get the extended error context). The `getSockErr()`, `setSockErr` and `strSockError()` functions are still used in the `netsocket.cpp` implementation, but now they are strictly confined to this particular translation unit, meaning they have now become an implementation detail, rather than a part of public API contract of `netplay` library. Signed-off-by: Pavel Solodovnikov <[email protected]>
ManManson
force-pushed
the
netsocket_api_expected_refactoring
branch
from
October 13, 2024 11:20
95c25c2
to
6337be0
Compare
past-due
approved these changes
Oct 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Switch to returning socket error codes explicitly via return values instead of using legacy functions
getSockErr()
,setSockErr
,strSockError()
to get extended error context.To facilitate that, use
tl::expected<T, std::error_code>
as the return value type for all main socket operations: read, write, open, listen.Moving to this new approach has numerous benefits over the old one:
Instead of using POSIX constants directly, we wrap them into
std::error_code
instances, which allows to attach custom error categories to them. This can be used to customize error messages and error codes mapping to platform-independentstd::error_conditions
.Calling separate
get/setSockErr()
functions is error-prone: one can easily forget to check the error condition fromgetSockErr()
and the value will be overwritten by the next socket function without the ability to recover the former error.Conversely, one can forget to call
setSockErr()
to set the proper error code for the caller to check upon.As mentioned above,
std::error_code:s
can be implicitly mapped to platform-independentstd::error_conditions
, allowing for this code to compile successfuly:if (errCode == std::errc::connection_reset) { ... }
This allows for very convenient and portable error checking code, which completely hides implementation details of how a particular error code is implemented (but, if one really needs to, they still can extract the platform-dependent error code value to get the extended error context).
The
getSockErr()
,setSockErr
andstrSockError()
functions are still used in thenetsocket.cpp
implementation, but now they are strictly confined to this particular translation unit, meaning they have now become an implementation detail, rather than a part of public API contract ofnetplay
library.Signed-off-by: Pavel Solodovnikov [email protected]