Hi Uday,
see my reply interim with your code.
On Tue, 31 Jan 2017 15:31:59 +0530
Uday Vernekar <vernekaru...@gmail.com> wrote:
> Requirement:
>
> I have Test log files with filename as SerialNumber of tested Product and
> the log file consist of
> below data and other Text.the IP addresses can be repeated need to fetch
> only mac starting with 00:05:95:XX:XX:XX
>
> <DATA>
> Board IP address : 192.168.1.1:ffffff00
> Host IP address : 192.168.1.100
> Gateway IP address :
> Run from flash/host/tftp (f/h/c) : f
> Default host run file name : vmlinux
> Default host flash file name : bcm963xx_fs_kernel
> Boot delay (0-9 seconds) : 1
> Default host ramdisk file name :
> Default ramdisk store address :
> Board Id (0-5) : 968500CHERRY
> Number of MAC Addresses (1-32) : 10
> Base MAC Address : 00:05:95:37:17:8b
> PSI Size (1-64) KBytes : 24
> Enable Backup PSI [0|1] : 0
> System Log Size (0-256) KBytes : 0
> Auxillary File System Size Percent: 0
> Main Thread Number [0|1] : 0
> MC memory allocation (MB) : 0
> TM memory allocation (MB) : 0
> <DATA>
>
> The data in the Test logs get appended everytime it is tested.
>
> I need to fetch "Base MAC Address : 00:05:95:37:17:8b"
> from the each and evry logfile and put in another file with Respect to
> SerialNumber of the Product.
> If it doesnt find any Mac then it should display MAC NOT PROGRAMMED for
> that SerialNumber.
>
> for Ex output in the file:
> Serial Number Mac Programmed
>
> AG-0117-10-1740 00:05:95:37:17:8b
> AG-0117-10-1741 00:05:95:37:17:8c
> AG-0117-10-1742 00:05:95:37:17:8d
> AG-0117-10-1743 MAC NOT PROGRAMMED
> AG-0117-10-1744 00:05:95:37:17:8e
>
> I Have written the code Please help me correcting the Errors which I have
> made in my code.
>
> <Start Code>
>
> #!/usr/bin/perl
> use strict;
> use warnings;
It's good that you are using those.
> use Cwd;
Perhaps import getcwd/etc. explicitly or do «use Cwd ();».
>
> print "\tEnter the User Name:";
> my $user = <STDIN>;
> chomp($user);
> print "\tEnter the Directory name where Logs are Stored:";
> my $Directory = <STDIN>;
> chomp($Directory);
You have some duplicate code here.
> system("mkdir -p /mnt/ONT");
You should use https://metacpan.org/pod/File::Path or whatever instead of
shelling to system which is less portable and may be slower. See:
http://perl-begin.org/tutorials/bad-elements/#calling-the-shell-too-much
(Note : I maintain that page).
> my $userPath="/mnt/backup/$Directory";
> chomp($userPath);
>
No need for chomp here.
> my $scriptfile = "/mnt/ONT/MAC-REPORT-FILE.txt";
> system("rm -f $scriptfile");
again - you should use unlink.
>
> open(SCRIPT , ">>$scriptfile");
>
See http://perl-begin.org/tutorials/bad-elements/#open-function-style - you can
also try using https://metacpan.org/pod/autodie .
> print "\n Results are stored in /mnt/ONT/MAC-REPORT-FILE.txt\n";
>
> `rm /mnt/ONT/MAC-REPORT-FILE.txt`;
>
1. Use unlink().
2. Don't use backticks instead of system -
http://perl-begin.org/tutorials/bad-elements/#qx_for_command_execution
> sub ScanDirectory {
> my ($workdir) = shift;
>
You should do either:
my ($work_dir) = @_;
or:
my $work_dir = shift;
See scalar vs. list context.
Also see http://perl-begin.org/tutorials/bad-elements/#calling-variables-file
> my($startdir) = &cwd;
>
Don't call subroutines using ampersands :
* http://perl-begin.org/tutorials/bad-elements/#ampersand-in-subroutine-calls
* https://perlhacks.com/2015/04/subroutines-and-ampersands/
> chdir($workdir) or die "Unable to enter dir $workdir:$!\n";
I find that using chdir is usually a bad idea.
>
> opendir(DIR, "$workdir") or die "Unable to open $workdir:$!\n";
> my @names = readdir(DIR);
> closedir(DIR);
>
> foreach my $name (@names){
> next if ($name eq ".");
> next if ($name eq "..");
>
You want to use
http://perl-begin.org/tutorials/bad-elements/#no_upwards_for_dirs
Also see http://perl-begin.org/tutorials/bad-elements/#flow-stmts-without-labels
> if (-d $name){
> my $pwd=`pwd`;
why not use Cwd.pm?
> chomp($pwd);
> my $path=$pwd."/".$name;
> &ScanDirectory($path);
> next;
> }
> unless (&CheckFile($name)){
>
Seems like you want to use File::Find or equivalent here -
http://perl-begin.org/tutorials/bad-elements/#recursive_directory_traversal
> }
> }
>
> close(SCRIPT);
> chdir($startdir) or die "Unable to change to dir $startdir:$!\n";
> }
>
> my $date =`date`;
use localtime() - http://perldoc.perl.org/functions/localtime.html
> my $data_file2='/mnt/ONT/MAC-REPORT-FILE.txt';
> open DATAOUT, ">>$data_file2" or die "can't open $data_file2 $!";
> print DATAOUT "\n\t\t\tMAC-ID GENERATED REPORT\n";
> print DATAOUT "\nGeneteated By:$user\n";
you misspelled "Generated".
> print DATAOUT "\nDATE: $date \n";
> print DATAOUT "NOTE: THIS REPORT IS GENERATED BASED ON THE TEST LOGS
> AVAILABLE IN THE SERVER\n";
> print DATAOUT "\n";
> print DATAOUT " SERIAL NUMBER\t\tMACADDRESS\n";
> print DATAOUT "-----------------------------------------\n";
> close(DATA);
>
> sub CheckFile{
> my($name) = shift;
>
> my $datafile=$name;
> #print "$datafile\n";
> my $data_file2='/mnt/ONT/MAC-REPORT-FILE.txt';
>
> open DATA, "$datafile" or die" can't open $datafile $!";
> my @array_of_data=<DATA>;
>
> #print @array_of_data;
>
> foreach my $i ($datafile)
> {
You're iterating over a scalar instead of a list.
>
> my $Result = `grep "Base MAC Address" \/mnt\/backup\/$Directory\/$name |
> tail -n 1 `;
don't use /bin/grep when you can use perl's file and text processing tools.
> my ($str,@mac) = split(":",$Result);
> if($mac[1] ne "05") {
>
> @mac=" MAC NOT PROGRAMMED\n";
> #print "\n";
> print DATAOUT " $name\t\t@mac\n" ;
> }
> else{
> print DATAOUT " $name\t\t
> $mac[0]:$mac[1]:$mac[2]:$mac[3]:$mac[4]:$mac[5]\n" ;
> }
> }
> }
>
> &ScanDirectory($userPath);
>
> close(DATAOUT);
>
>
> <END CODE>
>
>
In short - this code should really be refactored, corrected, and modernised.
Perhaps you should read http://www.onyxneon.com/books/modern_perl/index.html or
a similar book or tutorial - see http://perl-tutorial.org/ and
http://perl-begin.org/ .
Regards,
Shlomi Fish
--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/
