-
Notifications
You must be signed in to change notification settings - Fork 3k
SocketAddress rework #12683
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
SocketAddress rework #12683
Conversation
* Add optimised constexpr default constructor. Default construction was previously by a heavyweight defaulted `nsapi_addr_t` parameter. * Remove deprecated resolving constructor. * Take `nsapi_addr_t` inputs by constant reference rather than value. * Inline the trivial getters and setters. * Use `unique_ptr` to manage the text buffer. * Make `operator bool` explicit. * Optimise some methods. * Update to C++11 style (default initialisers, nullptr etc)
As previous PR was approved, although I made a comment there about having one commit for this rework is not ideal for future maintenance. @kivaisan Would you be able to split the first commit ? I started CI meanwhile |
@kivaisan, thank you for your changes. |
Test run: FAILEDSummary: 1 of 6 test jobs failed Failed test jobs:
|
Test failed, but I saw this recently in another PR.
@ARMmbed/mbed-os-storage Please review, looks like this is on master as well. |
Same failure as in the minimal-printf PR. |
Just read your comment there, I notified the test team to check the board. |
test restarted |
Summary of changes
Original work: #12468 . This PR contains also the UT fix.
nsapi_addr_t
parameter.nsapi_addr_t
inputs by constant reference rather than value.unique_ptr
to manage the text buffer.operator bool
explicit.Impact of changes
bool
orint
or others no longer possible - any existing code which does not compile is most likely an error. (if (sockaddr)
is still fine - such "contextual conversions to bool" can use the explicit operator).Migration actions required
SocketAddress
's constructor must be modified to useNetworkInterface::gethostbyname
orNetworkStack::gethostbyname
.Documentation
n/a
Pull request type
Test results
Reviewers