There has to be a better way - script loaded with system calls 4

[stfaprc]

below is an inherited script, done sometime in the mid 90s.
it works, but seems very inefficeint - especially all the system calls to do file work. Any hints on doing this better would be appreciated.
_ _ __ _ __ _ __ _ __ _ __ _ __ _ __ _ _

#!/usr/bin/perl
print "starting up 1602email \n";

system "ls -1 /mnt/AUTOMAIL/dailywebreport*.* > /tmp/$$";
open EMAIL,"</tmp/$$" or die "unable to open file $!";
my ($ifile, $ofile, $fromaddr, $basename, $aname, $base);
while(<EMAIL>)
{
  chomp;
  $ifile="$_";
  my $size = -s $ifile;
  if($size > 1)
  {
  my $attaches = 0;
  if($_ ne "ATTACH")
  {
    open IFILE,"$ifile" or die "cant open '$ifile'  $!";
    $ofile="$_.mail";
    open OFILE,">$ofile" or die "cant do '$ofile' $!";
    $fromaddr='PostMaster';
    while(<IFILE>)
    {
      chomp;
      if(substr($_,0,3) eq '@@@')
      {
        chomp($basename=substr($_,3,(length $_)-4));
        $aname="/mnt/automail/ATTACH/" . $basename;
        if(not -e "$aname")
        {
          print "$ifile is missing attachment \n";
        }
        $base="$aname.base64";
        system "/usr/bin/mimencode -o $base <$aname";
        print OFILE "\n--Message-Boundary--\n",
               "Content-type: Application/octet-stream;name=\"$basename\"\n";
        print OFILE "Content-transfer-encoding: BASE64\n\n";
        close OFILE;
        system "cat $base >> $ofile";
        open OFILE,">>$ofile";
        system "cp $base /usr/mailAttached/`date +%d`/";
        system "rm -f $base";
        system "rm -f $aname";
        $attaches=1;
      };
      if(substr($_,0,5) eq "From:")
      {
       $fromaddr=(substr($_,5));
      };
      if(substr($_,0,3) ne "\@\@\@")
      {
        print OFILE $_ ."\n";
      };
    }
    close IFILE;
    system "\\rm -f $ifile";
    if($attaches==1)
      {  print OFILE "\n--Message-Boundary----\n";
         $attaches=0;
      };
    close OFILE;
    system "cat $ofile | /usr/bin/sendmail -t -f$fromaddr";
    system "\\mv $ofile /mnt/mailgen/";
  }
  } #check of file size
}
close EMAIL;

 

[TonyGroves]

You could probably (I haven't examined the detail) rewrite it as a shell script, making use of sed for the string processing. But, if it ain't broke, is there any reason to risk breaking it?

 

[wardy66]

The rm commands can be replaced with unlink
The cp commands - hmmm, probably File::Copy
mv can be a copy and an unlink
ls can be opendir

etc etc

but you'd probably only bother for fun, as TonyGroves says

 

[travs69]

I think that File::Copy has a mv option also

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Travis - Those who say it cannot be done are usually interrupted by someone else doing it; Give the wrong symptoms, get the wrong solutions;

 

[travs69]

I would handle your system calls one at a time and make small changes.

Instead of system call to ls check out opendir and readir.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Travis - Those who say it cannot be done are usually interrupted by someone else doing it; Give the wrong symptoms, get the wrong solutions;

 

[spookie]

Hi stfaprc,
Instead of using system call you can use a backtick operator and store the output in an array instead of a file, thus bypassing file operation overhead.
e.g.

my @EMAIL = `ls -1 /mnt/AUTOMAIL/dailywebreport*.*`;

--------------------------------------------------------------------------
I never set a goal because u never know whats going to happen tommorow.

 

[travs69]

backticks are still system calls though. And a lot of what is going on isn't just sending data to a file, it's other file operations (copy's, enconding and such). I think he would pick up a lot more performance just to work through and do away with as many as possible.

I would also suggest keeping a original version of the code and benchmark it or at least use time to see how long it runs and then test the new version. It give you an idea of how much faster your script is once you do away with system calls.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Travis - Those who say it cannot be done are usually interrupted by someone else doing it; Give the wrong symptoms, get the wrong solutions;

 

[stfaprc]

The reason I am looking to replace is that the number of files that are to be processed has increased quite a bit since the script was first written. And this is running on a Pentium III 550Mhz pc with RH9.

 

[stfaprc]

some specific questions:

system "cat $base >> $ofile";

Is this a useless use of cat ? is there a better way of appending the contents of $base to $ofile ?

        system "rm -f $base";
        system "rm -f $aname";

Can this be made into:
system "rm -f $base; rm -f $aname";
to make the process smoother ?

if(substr($_,0,5) eq "From:")

Is there a faster way to see if a line is
starting with "From:" ?

 

[TonyGroves]

> Is this a useless use of cat ?
Not at all.

>Can this be made into: system "rm -f $base; rm -f $aname";
>to make the process smoother ?
You could use either:[tt]
system "rm -f $base $aname";[/tt]
or[tt]
unlink($base,$aname);[/tt]
unlink is probably faster.

>Is there a faster way to see if a line is starting with "From:" ?
You could use a regex:[tt]
if(/^From:/)[/tt]
Don't know if it's faster though.

 

[travs69]

this
system "rm -f $base";
system "rm -f $aname";
can be changed to

unlink $base;
unlink $aname;

You should really add the full path also (maybe at a variable at the top.

The substr is not going to be a big slow down compared to how many file operations you are doing.


I'm not sure what the equivalant of this is in perl
"/usr/bin/mimencode"
as I don't use it. Barring that I don't see a reason for system calls in your script.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Travis - Those who say it cannot be done are usually interrupted by someone else doing it; Give the wrong symptoms, get the wrong solutions;

 

[ChrisHunt]

I'd take a step back and look at what this script is intended to achieve, rather than converting each system call into its perl equivalent.

What your script appears to do is look through the files in a particular directory and email (some?) of them to some address.

You should (I think) be able to do that without any explicit system calls, using perl's directory/file handling commands and one of the Mail:: or EMail:: modules to handle the nuts & bolts of the emailing. It may or may not be more efficient (it's doing the same job, after all), but it should be much easier to maintain.

-- Chris Hunt
Webmaster & Tragedian
Extra Connections Ltd

 

[stfaprc]

I'm trying spookie's suggestion, which I like since it cuts down of file io.
This is what I did:

my ($ifile, $ofile, $fromaddr, $basename, $aname, $base);

my @EMAIL = `ls -1 sendemail*.*`;
foreach $ifile (@EMAIL)
{
  print "$ifile \n";
  my $size = -s $ifile;
  print "size = $size \n";

problem is that $size does not get assigned any value.
when I put it through debug mode, x $size results in undef.

What do I have incorrect?

 

[wardy66]

Does it print the file name ok?

No trailing carriage return in there?

If on Unix, run the script and pipe through "cat -vet"

Eg,
perl script.pl | cat -vet

 

[stevexff]

On my box ls -l something* doesn't produce a nice parseable list of files.

use strict;
use warnings;

my @dirlist = qx(ls -l);

print $_ foreach (@dirlist);

gives me

total 72
-rw-rw-r-- 1 steve steve 107 2008-01-29 08:30 master.txt
-rw-rw-r-- 1 steve steve 107 2008-01-29 08:08 master.txt~
-rw-rw-r-- 1 steve steve 103 2008-02-05 18:18 sortlist.txt
-rw-rw-r-- 1 steve steve 428 2008-01-29 09:28 switch.pl
-rw-rw-r-- 1 steve steve 447 2008-01-29 08:51 switch.pl~
-rw-rw-r-- 1 steve steve  66 2008-01-29 08:30 switch.txt
-rw-rw-r-- 1 steve steve  66 2008-01-29 08:27 switch.txt~
-rwxrwxr-x 1 steve steve  82 2008-02-21 07:55 test.pl
-rw-rw-r-- 1 steve steve 638 2008-02-20 08:33 test.pl~

If yours is the same, it could be part of the problem...

Steve

[small]"Every program can be reduced by one instruction, and every program has at least one bug. Therefore, any program can be reduced to one instruction which doesn't work." (Object:erlDesignPatterns)[/small]

 

[spookie]

stfaprc,

Does it require to give an absolute path ?

my $absPathifile = "/pathtoifile/".$ifile ;
 my $size = -s $absPathifile;

--------------------------------------------------------------------------
I never set a goal because u never know whats going to happen tommorow.

 

[stfaprc]

okay, it was a CR problem.
changing to

my @EMAIL = `ls -1 /mnt/opserve/mailgen/sendemail*.*`;
chomp @EMAIL ;

results in the my $size working. (note that the ls option is the number one and not lower case L)

now I should figure out how to give a non-root user rw access to a smbfs mount done in /etc/fstab