Baunsgaard commented on code in PR #2586:
URL: https://github.com/apache/systemds/pull/2586#discussion_r4066170462
##########
bin/systemds:
##########
@@ -173,6 +173,32 @@ script.
EOF
}
+# verify that $1 is a port that a server can be bound to, otherwise abort with
an error.
+# $2 names the server the port is meant for.
+function checkPort {
+ local port=$1
+ local target=$2
+ local re='^[0-9]+$'
+ if ! [[ $port =~ $re ]] ; then
+ echo "error: Port '$port' for the $target is not a number"
+ printUsage
+ exit 1
+ fi
+ # drop leading zeros, so that the length check below is not fooled by e.g.
0000080505
+ local num=$port
+ while [[ ${#num} -gt 1 && $num == 0* ]] ; do num=${num#0} ; done
+ # more than 5 digits is out of range by definition, and comparing it would
silently
+ # overflow the 64 bit integers of the shell for very long inputs
+ if [ ${#num} -gt 5 ] || [ "$num" -lt 1 ] || [ "$num" -gt 65535 ] ; then
+ echo "error: Port $port for the $target is out of range, expected a port
in [1, 65535]"
+ printUsage
+ exit 1
+ fi
+ if [ "$num" -lt 1024 ] ; then
+ echo "warning: Port $port for the $target is a reserved system port,
binding it requires elevated privileges"
+ fi
+}
+
Review Comment:
Only criticism. I am strongly against adding more logic to the systemds
file. Can we not just have the error message and verification inside Java. This
gives a single point of failure, rather than splitting the logic in two.
While there was some logic already verifying basic number compatibility, i
think that should also just be cleaned up and moved into the java PortUtils you
have defined.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
