Move port and cport range check just before clusterStartHandshake function

[hwware]

Now for the cluster meet command, we first check the port and cport number, and then the function clusterStartHandshake() is called.
In the function clusterStartHandshake(), ip address is checked first and then port and cport range are checked.
Even port or cport range is out of bound, only error message "Invalid node address specified" is reported.
This is not correct.

In this PR, I just make the port and cport check together before clusterStartHandshake() function. Thus the port number and range can be checked together.

[codecov]

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 71.17%. Comparing base (79d5047) to head (e634138).
Report is 2 commits behind head on unstable.

Files with missing lines	Patch %	Lines
src/cluster_legacy.c	33.33%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           unstable    #1686   +/-   ##
=========================================
  Coverage     71.17%   71.17%           
=========================================
  Files           123      123           
  Lines         65641    65644    +3     
=========================================
+ Hits          46720    46724    +4     
+ Misses        18921    18920    -1     

Files with missing lines	Coverage Δ
src/cluster_legacy.c	86.12% <33.33%> (+0.22%)	⬆️

... and 13 files with indirect coverage changes

[xbasel]

This function does more than just verification, it also parses and assigns port values. Consider renaming it to something like parseAndValidatePorts for better clarity.

[hwware]

It makes sense to me. Will update.

[zuiderkwast]

The behavior change is that CLUSTER MEET can return an error instead of OK when the ports are invalid. This means the user can see the error early and understand what's wrong.

This is the user-visible change, so it should be the title of the PR I think. The rest is just refactoring which is not important for users and release notes.

[enjoy-binbin]

why we need this? The old code also seems to be very easy to understand and maintain

[hwware]

why we need this? The old code also seems to be very easy to understand and maintain

Current old cause developers hard to identify the problem. Existing logic is to check port,, and then in function clusterStartHandshake check the ip first and check the port again.

The easier way to put all port checking logic together, and then check the ip.

[zuiderkwast]

@enjoy-binbin @hwware The first commit is smaller. It just moves the check from clusterStartHandshake to CLUSTER MEET so it can return an error, without extra refactoring.

ce4c0d4

[hwware]

@enjoy-binbin @hwware The first commit is smaller. It just moves the check from clusterStartHandshake to CLUSTER MEET so it can return an error, without extra refactoring.

ce4c0d4

Yes, I just make thing easier. It looks like i update codes to the fist commit, and check the port and cport in the Cluster Meet part.
Sometimes simple is good enough

[zuiderkwast]

Yes, this is simpler. I like it more. @enjoy-binbin WDYT?

I have just a minor comment about the error message.

[zuiderkwast]

LGTM.

Before merge, please update the PR title to clarify the user-visible behavior, that CLUSTER MEET validates the ports and returns error for invalid ports.

That is what we should mention in the release notes.