Apply option local port #222
Merged
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
mrdeep1
reviewed
Jan 18, 2024
tests/dtls-client.c
Outdated
Comment on lines
489
to
494
| case -1 : | ||
| /* handle arguments */ | ||
| if (!dst.size) { | ||
| /* resolve destination address where server should be sent */ | ||
| res = resolve_address(argv[optind++], &dst.addr.sa); |
There was a problem hiding this comment.
I'm not convinced that this is the best way do do this (very unusual), moving the logic processing the arguments to within the getopt() logic, and fail to see why it is actually needed. If this is done, at a minimum dtls_set_log_level(log_level); needs to be included at the start.
I think that simply removing the use of port_str, using the new local_port or DEFAULT_PORT in the appropriate places along with the other changes should should work
There was a problem hiding this comment.
Ah - yes, this really should be against #221.
There was a problem hiding this comment.
I already copied your comment ;-)
boaks
force-pushed
the
apply_option_local_port
branch
from
daec394
to
452fb25
Compare
boaks
force-pushed
the
apply_option_local_port
branch
from
452fb25
to
3264aa9
Compare
boaks
force-pushed
the
apply_option_local_port
branch
from
3264aa9
to
a8473ea
Compare
boaks
force-pushed
the
apply_option_local_port
branch
from
a8473ea
to
0a6d10a
Compare
obgm
approved these changes
Aug 28, 2024
boaks
force-pushed
the
apply_option_local_port
branch
from
0a6d10a
to
03489f7
Compare
|
I guess, adding a ',' will be an improvement :-) |
The previous version ignores the option for the local port. That may be caused by issues using the same default local port for the server and client. This enables the use of an specific local port and changes the default to an ephemeral free port, similar to quite a lot of other UDP clients. The DEFAULT_PORT is therefore only used for the destination. Signed-off-by: Achim Kraus <achim.kraus@cloudcoap.net>
boaks
force-pushed
the
apply_option_local_port
branch
from
03489f7
to
559f37d
Compare
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.
The previous version ignores the option for the local port. That may be
caused by issues using the same default local port for the server and
client.
This enables the use of an specific local port and changes the default
to an ephemeral free port, similar to quite a lot of other UDP clients.
The DEFAULT_PORT is therefore only used for the destination.