in reply to RFC: Net::SNTP::Server v1

1) know your tools 
    # Convert the decimal to hexadecimal and pad the result with a lea
+ding
    # zero if needed.
    my $hex = sprintf("%x", $dec);
    $hex = length( $hex ) == 1 ? "0".$hex : $hex;

[download]
way too complicated 
    my $hex = sprintf '%02x', $dec;

[download]
conversion table 
  0 -> 00
  1 -> 01
 15 -> 0f
 20 -> 14
255 -> ff
256 -> 100    # garbage in, garbage out
2) nesting subs does not work: 
sub basicSNTPSetup {
  ...   
  sub keyDifference {
    ..
  };
};

[download]
is equivalent to 
sub basicSNTPSetup {
  ...   
};

sub keyDifference {
  ..
};

[download]
3) dead code / sub-ref / conditional 
my $verify_port = sub {
  my $port = shift;
  if ( defined $port && $port =~ /^[0-9]+$/ ) {
    if ( $port >= MIN_UDP_PORT || $port <= MAX_UDP_PORT ) {
      return FALSE;
    }
  }
  return TRUE;
};

[download]
This functions does not seem to be referenced anywhere in your code. Additionally: Why do you code it as a sub reference instead of an actual function? 
sub verify_port {
  ...
};

[download]
inside verify_port you are using the wrong conditional 
  if ( $port >= MIN_UDP_PORT || $port <= MAX_UDP_PORT ) {

[download]
This will happily allow 10000000000 as a valid port. At the very least you should replace || with &&. I'm to lazy too look up operator precedence, but you may also need to replace && with 'and' or put the conditions into parenthesis. 
either:
  if ( ($port >= MIN_UDP_PORT) && ($port <= MAX_UDP_PORT) ) {
or:
  if ( $port >= MIN_UDP_PORT and $port <= MAX_UDP_PORT ) {

[download]

Resumee:

I would not want to touch this module with a ten-foot pole. I wasn't even looking very hard.

Replies are listed 'Best First'.
Re^2: RFC: Net::SNTP::Server v1
by thanos1983 (Parson) on Jul 17, 2015 at 23:17 UTC

    Hello Monk::Thomas,

    First of all thank you for your time and effort reading and reviewing my code. I am not an expert and it is good that you point out this error to me so I can modify the code and updated to a better hopefully version.

    Regarding Nested Functions, I found this tutorial Creating Nested Functions, from where I modified my functions to local hopefully this is better now and more memory efficient.

    Regarding your comment:

    I would not want to touch this module with a ten-foot pole. I wasn't even looking very hard.

    Is is to so badly written the module? What is so wrong that would make you not want to use this module?

    Again, thank you for time and appreciate your effort.

    Seeking for Perl wisdom...on the process of learning...not there...yet!
[reply]
[d/l]
[select]

      Regarding Nested Functions, I found this tutorial Creating Nested Functions, from where I modified my functions to local hopefully this is better now and more memory efficient.

      If you're concerned about optimized memory consumption for an 8 line function, then you probably should use something else. (e.g. finely tuned C code) My recommendation: Optimize for readability / maintainability. You like to publish the module so that someone else might use it. This someone else might have an anger management problem, an axe and your address. ;)

      Is is to so badly written the module? What is so wrong that would make you not want to use this module?

      It's trust related. If I use someone else's code then I must be sure the code is doing what it's supposed to do. Easy to understand code and suitable application of common language idioms help to build that trust. (Hey! I can understand that!) Automated tests are also nice. (Hey! I don't understand it, but it's seems to be working as expected.) Your code simply fails to build that trust.

      'Three strikes and you are out:'

      1. Trying to reinvent printf, especially even though you already use it.
      2. Code that does not seem to be used.
      3. Failing to validate user input.

      No trust -> that code is not going to end up on any of my machines. I may want to play around with it for educational purposes, but it's not going to be something I rely on.

[reply]

In Section Meditations