Karyn Williams wrote:
> I have this script I have been working on where I need to redirect STDOUT
> and STDERR to files at the beginning of the script and then back to default
> (terminal) at the end of the script. I found an example on-line and used it
> and it works, however it generates a warning when running my script.

Try using the example that comes with Perl:

perldoc -f open

> I would prefer to make that warning go away. This is the output of the script:
> 
> alum# ./match.pl
> Name "main::OLDERR" used only once: possible typo at ./match.pl line 38.
> Name "main::OLDOUT" used only once: possible typo at ./match.pl line 37.
> Records processed: 2
> Kicked as dups: 1
> Kicked as exceptions: 0
> Accounts created: 1
> 
> This is the script:
> 
> #!/usr/bin/perl
> 
> # match.pl expanded
> # Checks the current csv against the passwd file to detect and prevent
> creating duplicate accounts
> 
> use strict;
> use warnings;
> 
> my $dir = "/usr/local/accounts";
> 
> my $csv = "isirin.csv";
> my $expin = "exceptions.csv";
> my $pwfile = "/etc/passwd";
> my $expout = "exceptions.log";
> my $clog = "account_change.log";
> my $as400 = "as400.txt";
> my $admiss = "admissions.txt";
> my $err = "error.log";
> 
> my $description = "SIR Fall 2007";
> 
> my $date = localtime();
> my ($sec,$min,$hour,$mday,$mon,$year) = (localtime)[0,1,2,3,4,5];
> $mday++;

You are incrementing the day of the month so if today is 31 May your string
will display the 32nd day of May!


> $mon++;
> $year += 1900;
> my $today = sprintf("%04d%02d%02d-%02d%02d%02d", $year, $mon, $mday, $hour,
> $min, $sec);
> 
> # Read in csv file from Accounting and check for existing accounts.
> 
> open (IN, "$dir/$csv") or die "$dir/$csv does not exist. This file must be
> available to run this script: $!";
> open (EX, "$dir/$expin") or die "Can't open EX $expin : $!";
> open (PW, "$pwfile") or die "Can't open PW $pwfile : $!";
> open (EXCEP, ">>$dir/$expout") or die "Can't open EXCEP: $!";
> open (OUT, ">>$dir/$clog") or die "Can't open OUT: $!";
> open (AOUT, ">$dir/$as400") or die "Can't open AOUT: $!";
> open (OLDOUT, ">&STDOUT");
> open (OLDERR, ">&STDERR");
> open (STDOUT, ">$dir/$admiss") or die "Can't open STDOUT: $!";
> open (STDERR, ">>$dir/$err") or die "Can't open STDERR: $!";

You should use the three argument form of open and you should *ALWAYS* verify
that open succeeded:

open IN,     '<',  "$dir/$csv"    or die "$dir/$csv does not exist. This file
must be available to run this script: $!";
open EX,     '<',  "$dir/$expin"  or die "Can't open EX $expin : $!";
open PW,     '<',  $pwfile        or die "Can't open PW $pwfile : $!";
open EXCEP,  '>>', "$dir/$expout" or die "Can't open EXCEP: $!";
open OUT,    '>>', "$dir/$clog"   or die "Can't open OUT: $!";
open AOUT,   '>',  "$dir/$as400"  or die "Can't open AOUT: $!";
open OLDOUT, '>&', \*STDOUT       or die "Can't dup STDOUT: $!";
open OLDERR, '>&', \*STDERR       or die "Can't dup STDERR: $!";
open STDOUT, '>',  "$dir/$admiss" or die "Can't open STDOUT: $!";
open STDERR, '>>', "$dir/$err"    or die "Can't open STDERR: $!";


> my $pwf;
> my $exp;
> my $gecos;
> my $login;
> my $rcount = 0;
> my $acount = 0;
> my $ecount = 0;
> my $lcount = 0 ;
> 
> my @line = <IN>;

Why are you reading the entire file into memory if you don't really have to?

> printf STDOUT ("$date\n");

That should be print instead of printf:

print STDOUT "$date\n";

printf() requires that the first string is a format string and will
interpolate certain characters.


> # Slurp the /etc/passwd file into one string and then search for matching
> Student IDs in that 
> # string. Use this later for checking unused loginnames also.
> 
> while (<PW>) {
>         $pwf .= $_;
>         }

my $pwf = do { local $/; <PW> };


> while (<EX>) {
>         $exp .= $_;
>         }

my $exp = do { local $/; <EX> };


> # Create GECOS and login info and then create account using pw
> 
> foreach my $line (@line) {

You probably should use a while loop instead of a foreach loop.

>         next if $line =~ /^nameid/;
>         next if $line =~ /^$/;
>         next if $line =~ /^\s+$/;
>         # file format is nameid,first,init,last
>         #print $line;
>         chomp $line;
>         $rcount++;
>         my ($sid,$name) = split(/,/, $line,2);
>         if ($pwf =~ /\b$sid\b/) {

You should probably store the $sid values in a hash.

>                 printf EXCEP ("$date, $name, $sid, possible dup

You should use print instead of printf.

> account\n"); $acount++;
>                 } elsif ($exp =~ /\b$sid\b/) {

You should probably store the $sid values in a hash.

>                 printf EXCEP ("$date, $name, $sid, listed in

You should use print instead of printf.

> exceptions.csv\n"); $ecount++;
>                 } else {
> 
>         my ($first, $initial, $last) = split(/,/, $name, 3);
>         # Create gecos info
>         $last =~ s/[",]//g;
>         $first =~ s/[",]//g;
>         $initial =~ s/[",]//g;

          tr/",//d for $first, $initial, $last;


>         if ($initial) { 
>                 $gecos = "\"$first $initial $last, $sid, $description\"";
>                 } else {
>                 $gecos = "\"$first $last, $sid, $description\""; }
>         #print "GECOS IS $gecos\n";
>         # Now guess at a login
>         my $lfirst = $first;
>         $lfirst =~ s/[^a-zA-Z]//g;
>         #(my $lfirst = $first) =~ s/[^a-zA-Z]//g);
>         my @last = split(/\s/, $last, 2);
>         my $llast = $last[0];

You don't need the array if you are only extracting one value:

         my $llast = ( split ' ', $last )[ 0 ];

>         $llast =~ s/[-\s]//g;
>         #print "LoginLast is $llast\n";
>         if (length($lfirst) + length($llast) <= 16) {
>                 $login = $lfirst . $llast; 
>                 } else {
>                 my $f = substr(($lfirst),0,1);
>                 $login = $f . $llast;
>                 }
>         $login = lc($login); 
>         #print "Login is $login\n";
>         #printf STDOUT ("$first $initial $last, $sid, $login, passwd\n");
>         printf STDOUT ("$first $initial $last, $sid, $login, ");

You should use print instead of printf.

>         #This next line is for testing purposes
>         system("pw useradd -n $login -c $gecos -m -s /bin/ftpshell -N");

You should verify that 'pw' executed correctly:

        system( 'pw', 'useradd', '-n', $login, '-c', $gecos, '-m', '-s',
'/bin/ftpshell', '-N' ) == 0 or die "Cannot run 'pw' $?";


>         #system("pw useradd -n $login -c $gecos -m -s /bin/ftpshell -w
> random");
>         printf OUT ("$date Account Added, $first $initial $last, $login,

You should use print instead of printf.

> $sid\n");
>         printf AOUT ("$sid, $first $initial $last, $login\n");

You should use print instead of printf.

>         $lcount++;
>         }
> }
> 
> close IN;
> close OUT;
> close AOUT;
> close STDOUT;
> close STDERR;
> close EX;
> close PW;
> close EXCEP;
> 
> rename ("$dir/$csv", "$dir/$csv.$today") or die "Can't rename $dir/$csv : $!";
> 
> open(STDOUT, ">&OLDOUT");
> open(STDERR, ">&OLDERR");

You should use the three argument form of open and you should *ALWAYS* verify
that open succeeded:

open STDOUT, '>&', \*OLDOUT or die "Cannot dup OLDOUT: $!";
open STDERR, '>&', \*OLDERR or die "Cannot dup OLDERR: $!";


> print "Records processed: $rcount\n";
> print "Kicked as dups: $acount\n";
> print "Kicked as exceptions: $ecount\n";
> print "Accounts created: $lcount\n";
> 
> # send via e-mail output of this script.
> 
> my $to = "To: [EMAIL PROTECTED]";
> my $from = "From: [EMAIL PROTECTED]";
> my $subject = "Subject: Test Output for SIRs";
> my $sendmail = "/usr/sbin/sendmail -t";
> 
> my @files = ("$dir/$as400", "$dir/$admiss");
> my @mail = ();
> 
> foreach my $file (@files) {
> 
> open (MAIL, "|$sendmail") or die "Can't open $sendmail: $!";
> open (AOUT, "<$file") or die "Can't open AOUT for read: $!";
> 
> print MAIL "$to\n";    
> print MAIL "$from\n";      
> print MAIL "$subject\n";         
> 
> @mail = <AOUT>;
>         foreach my $line (@mail) {
>                 print MAIL $line;
>         }

You don't need the array or loop there:

print MAIL "$to\n",
           "$from\n",
           "$subject\n",
           <AOUT>;


> close (MAIL);

When you close a piped open you should verify that it closed correctly:

perldoc -f close


> close AOUT;
> 
> }
> 
> # end of script


John
-- 
Perl isn't a toolbox, but a small machine shop where you can special-order
certain sorts of tools at low cost and in short order.       -- Larry Wall

-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/


Reply via email to