Hello Monk::Thomas,

Thank you again for your time and effort reviewing my code.

I can not understand why you recommend read

In further analysis, I am using the TRUE and FALSE in order to test for example if the port number is correct, which will result e.g. that the port is ok or maybe is not ok. Then based on the return (TRUE or FALSE) I have an if condition that will return $error if and only if the return from the function was TRUE.

Please correct me if I have miss understood the way you are thinking of using read.

read() is just an example for the thing I am recommending. Okay, let's take it from the top. Your code:

    my $verify_port = sub {
    my $port = shift;
    if ($port) {
        if ($port > MAX_PORT or $port < MIN_PORT){
        return TRUE;
        }
    }
    else {
        $moduleInput{-port} = 123;
        return FALSE;
    }
    };
[download]

If the port is outside the valid range, then you return true, if it is inside the allowed port range then you return (I am not sure - if no explicit return value is specified then the one from the last assignment is chosen - so - $port?) and if the port is undefined you set some value and return false.

btw. the '$moduleInput{-port} = 123;' thing is bad style because you're modifying some completely unrelated piece of data. It's a lot better to make it obvious when some value is changed.

This is how I would tackle this:

# I ~love~ readonly. Primarily because you can write stuff like:
# print "$MIN_TCP_PORT < x < $MAX_TCP_PORT\n";
use Readonly;

Readonly my $DEFAULT_NTP_PORT => 123;
Readonly my $MIN_TCP_PORT     => 1;
Readonly my $MAX_TCP_PORT     => 65535;

# in case of success: return a 'true' value   (1 or a useful value)
# in case of error:   return a 'false' value  (0 or undef)
sub verify_port {
  my $value = shift;

  # user provided input should be viewed as toxic - do not
  # trust until proven to contain a sane value
  if (defined $value and $value =~ /\A (\d+) \z/xms) {
    my $port = $1;

    if ($MIN_TCP_PORT <= $port and $port <= $MAX_TCP_PORT) {
      return 1;  # or TRUE or $port
    }
  }

  return 0;  # or FALSE or undef
}

# get a port value from somewhere, e.g. Getopt::Long
# use a default value if no value has been provided
my $input = $opt_ntp_port // $DEFAULT_NTP_PORT;

# - usage a -
if (verify_port($input)) {
  (do stuff with $input)
}
else {
  (issue warning / bail out)
}

# - usage b -
verify_port($input)
  or die;

# - usage c -
# (possible if verify_port() returns $port)
my $ntp_port = verify_port($input)
  or die 'Invalid ntp port!';
[download]

This is a similar behaviour to read() and that's why I mentioned it.

Usage c would not work if the returned value can be 0. Luckily 0 is not an allowed port for TCP!

Hello Monk::Thomas,

Finally I understood your comments, I was not able to understand that you first need to check if the port number is actually is an Integer:

if (defined $value and $value =~ /\A (\d+) \z/xms)
[download]

Well a few things that I noticed is that you need to modify the second condition from:

if ($MIN_TCP_PORT <= $port and $port <= $MAX_TCP_PORT)

to:

if ($MIN_TCP_PORT <= $port and $port >= $MAX_TCP_PORT)

The port should not be greater than the MAX_TCP_PORT not minor. ;)

Question, why are you assigning the $value to a new value $port = $1. Is there a specific reason?

Last modification that I applied on my updated version, is the port. You are right just by modifying the hash would not be clear to someone else on the future.

So I added a simple or condition when I am creating the socket. Sample of modification:

PeerPort => $moduleInput{-port} || DEFAULT_NTP_PORT, # Default NTP por
+t 123
[download]

Again thank you for your time and effort reviewing my code.

Seeking for Perl wisdom...on the process of learning...not there...yet!