Loading ...
Sorry, an error occurred while loading the content.

Re: [PBML] automatic NSlookup and Grep IP address

Expand Messages
  • Shlomi Fish
    Hi whysoserious, some comments on your code: On Thu, 06 Oct 2011 08:55:23 -0000 ... Always start with use strict; and use warnings; . See:
    Message 1 of 3 , Oct 6, 2011
    • 0 Attachment
      Hi whysoserious,

      some comments on your code:

      On Thu, 06 Oct 2011 08:55:23 -0000
      "whysoserious?" <chestersigua@...> wrote:

      > Hi, I'm a beginner in Perl. I'm self studying and I am trying out a simple program. Basically, I want to do a program which will automate nslookup and ping, then append the output to a file created on the start of the script. here is what i have so far:
      >

      Always start with "use strict;" and "use warnings;". See:

      http://perl-begin.org/tutorials/bad-elements/

      > if(-e "ipPool.out") {system("del ipPool.out")}
      >
      > system("cls");
      > print "Enter Controller Number : ";
      > $contNum = <STDIN>;
      > chomp($contNum);

      You should declare $contNum, and preferably not use camelCase for your
      identifiers.

      You may also wish to use "ARGV" instead of "STDIN".

      > print "Enter Port Number for : ";
      > $portNum = <STDIN>;
      > chomp($portNum);

      Again.

      > print "How many partitions to get : ";
      > $parNum = <STDIN>;
      > chomp($parNum);
      >

      You have some duplicate code here. Maybe extract a subroutine.

      > open(FILE, ">>ipPool.out") || die "Could not open Output File!";

      1. Use three args open.

      2. Use lexical filehandles.

      > print FILE "IP Pool for Partitions\n=====================\n\n";
      > $count = 1;
      > while ($count <= $parNum) {

      This is better done as a «for my $count (1 .. $parNum)» loop.

      > chomp($count);

      No need for this chomp.

      > my(@lookup) = `nslookup ss$contNum-$portNum-net7`;

      You're interpolating user-inputted variables into a command line command, which
      can cause "markup injection":

      http://community.livejournal.com/shlomif_tech/35301.html

      Furthermore, there's no need for the parentheses here.

      > $IPAddress = exec "ifconfig eth1 | grep 'Address:' | cut -d: -f2 | awk '{print $1}'";

      The exec will never return, and using grep, cut and/or awk from within perl is
      a red-flag, because these can easily be done using Perl code. "$1" will be
      interpolated by Perl here so it won't do what you want.

      You're not doing anything with $IPAddress. What is its significance?

      > print FILE "$contNum-$portNum\n======\n\n@lookup\n\n";

      If you're just outputting @lookup - why isn't it a string to begin with?

      > $count++;
      > }
      > close(FILE);
      > system("cls");
      > system("more ipPool.out");
      >

      These calls to system are not portable.

      Regards,

      Shlomi Fish


      --
      -----------------------------------------------------------------
      Shlomi Fish http://www.shlomifish.org/
      Best Introductory Programming Language - http://shlom.in/intro-lang

      Modern Perl — the 3‐D Movie. In theatres near you.

      Please reply to list if it's a mailing list post - http://shlom.in/reply .
    • Amish Husain
      on a side note indent your code to make it readable... http://en.wikipedia.org/wiki/Indent_style ________________________________ From: whysoserious?
      Message 2 of 3 , Oct 6, 2011
      • 0 Attachment
        on a side note indent your code to make it readable...

        http://en.wikipedia.org/wiki/Indent_style


        ________________________________
        From: whysoserious? <chestersigua@...>
        To: perl-beginner@yahoogroups.com
        Sent: Thursday, October 6, 2011 4:55 AM
        Subject: [PBML] automatic NSlookup and Grep IP address


         
        Hi, I'm a beginner in Perl. I'm self studying and I am trying out a simple program. Basically, I want to do a program which will automate nslookup and ping, then append the output to a file created on the start of the script. here is what i have so far:

        if(-e "ipPool.out") {system("del ipPool.out")}

        system("cls");
        print "Enter Controller Number : ";
        $contNum = <STDIN>;
        chomp($contNum);
        print "Enter Port Number for : ";
        $portNum = <STDIN>;
        chomp($portNum);
        print "How many partitions to get : ";
        $parNum = <STDIN>;
        chomp($parNum);

        open(FILE, ">>ipPool.out") || die "Could not open Output File!";
        print FILE "IP Pool for Partitions\n=====================\n\n";
        $count = 1;
        while ($count <= $parNum) {
        chomp($count);
        my(@lookup) = `nslookup ss$contNum-$portNum-net7`;
        $IPAddress = exec "ifconfig eth1 | grep 'Address:' | cut -d: -f2 | awk '{print $1}'";
        print FILE "$contNum-$portNum\n======\n\n@lookup\n\n";
        $count++;
        }
        close(FILE);
        system("cls");
        system("more ipPool.out");

        so basically, i want the script to ask how many partitions the user needs an ip address reserved. then the script will issue the nslookup command the number of times the user inputs, then append to an existing file created during program start.

        thanks!




        [Non-text portions of this message have been removed]
      Your message has been successfully submitted and would be delivered to recipients shortly.