Skip to content

SocketAddress: add port check to compare operator #11832

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

Closed
wants to merge 1 commit into from
Closed

SocketAddress: add port check to compare operator #11832

wants to merge 1 commit into from

Conversation

kivaisan
Copy link
Contributor

@kivaisan kivaisan commented Nov 7, 2019

Description (required)

Summary of change (What the change is for and why)

Port is considered as a part of socket address and compare operator
must also check if the port number is the same in both addresses.

Documentation (Details of any document updates required)

Pull request type (required)

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results (required)

PR includes also new unittest to verify SocketAddress compare operator. Also added a check to UDPSOCKET_ECHOTEST to verify the received packet is sent by echo server from echo port.

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[X] Tests / results supplied as part of this PR

Reviewers (optional)

@SeppoTakalo


Release Notes (required for feature/major PRs)

Summary of changes
Impact of changes
Migration actions required

Port is considered as a part of socket address and compare operator
must also check if the port number is the same in both addresses.

Added unittest and UDP echo test to compare addresses.
@ciarmcom ciarmcom requested review from SeppoTakalo and a team November 7, 2019 08:00
@ciarmcom
Copy link
Member

ciarmcom commented Nov 7, 2019

@kivaisan, thank you for your changes.
@SeppoTakalo @ARMmbed/mbed-os-test @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 7, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Nov 7, 2019

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-ARM

@mbed-ci
Copy link

mbed-ci commented Nov 7, 2019

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_unittests

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 7, 2019

Unittest failed, features-netsocket-nsapi_dns please review

@kivaisan
Copy link
Contributor Author

kivaisan commented Nov 7, 2019

This requires more thinking and can be also a breaking change for existing applications. I'll close this PR and make a new one which just tests address and port in echotest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants