in reply to Yet Another Tar-Gzip File Expander

A couple of things I notice... I might rewrite the code like this: 
#!/usr/bin/perl -w
$|++;
use strict;

my $total=0;
my $dir = shift || die "Needs Directory Param!\n";

opendir LOCAL,$dir or die "Can't open directory '$dir': $!\n";
while (my $file = readdir LOCAL)
{
    next unless $file =~ /\.t(?:ar\.)?gz\z/;
    system("tar", "xzvf", "$dir/$file");
    $total++;
}
closedir LOCAL;

print "$total files expanded!\n";

[download]
Update: fixed a typo as pointed out by straywalrus.

Replies are listed 'Best First'.
Re: Re: Yet Another Tar-Gzip File Expander
by straywalrus (Friar) on May 14, 2002 at 01:34 UTC
    I took your code and down loaded it, then I realized a few things:

    A) you left out the first brace on the line 
     next unless $file =~ /\.t(?:ar\.)?gz\z/);
# it should be
next unless ($file =~ /\.t(?:ar\.)?gz\z/);

    [download]
    That was not meant to flame you, just to point it out for others who read this.

    <ident>B) Your script runs into the same flaw as mine: It expands the archives in the CWD. I read the manual on tar, but it was older than tar, so the option I thought should work didn't. Really all I think I need to do is to change the current working directory to whatever $ARGV[0] is at the time. That should solve it. Thanx again for the regex compression. i re-wrote mine as this: 
    #!/usr/bin/perl -w
$|++;
use strict;
my (@list, $d, $i);
die "Need Directory Param!\n" unless $ARGV[0];
my $cmd = qq/tar -zxvf /;
opendir LOCAL,$ARGV[0] || die "Cannot Open dir $ARGV[0]";
@list = readdir LOCAL;
foreach $d (0..$#list)
{
    if($list[$d] =~ /.+\.t?(ar\.)?gz/)
    {
        qx/$cmd $ARGV[0]\/$list[$d]/;
        $i++;
    }
}
print "$i files expanded!\n";
closedir LOCAL;

    [download]
    Of Course this is still a few lines larger than yours, but you see, this is my code and I kept it how i originally wrote it. No offense to you, but that is your code. Besides the fact that I did not explain my whole purpose in writing this program. I am going to expand this and need to have the directory read into an array for later use. Thanx again.

    <straywalrus>
[reply]
[d/l]
[select]

      I would prefer the system() over the qx//. Apart from being more secure, you can test whether the command was successful. At the moment, you are using qx// in void context : capturing the output of the command and ingoring it. This is a waste of memory and CPU cycles.

      I'd write it thus: 

       {
      system("tar", "zxvf", $ARGV[0] . "/$list[$d]") == 0
           or print STDERR "Error running tar zxvf on $list[$d]";
      $i++;
 }

      [download]

      my €0.02

[reply]
[d/l]
      Thanks for the correction. My code is untested, as you can clearly see. I hope you didn't take my criticisms as flames; publicly-posted code is meant to be scrutinized so that it can be improved. I have more to learn than most people when it comes to improving my code, anyway. :-)

      Oh, as far as the tar command only untar/zipping files in the current directory, you can change your current directory to the appropriate subdirectory by using the chdir command.

[reply]
        Don't worry I realize that you did not mean to flame me. And I was just flipping through "Perl In a Nutshell" and found chdir, but thanx a lot. And thanks to you I finnaly learned how to use the ?( )? for regex. Thanx for the code improvments. straywalrus
[reply]
Re: Re: Yet Another Tar-Gzip File Expander
by straywalrus (Friar) on May 13, 2002 at 22:33 UTC
    The security issue is not such a big issue, because this is on a stand alone Linux Box, but all the other issues will be changed. Thank you for reminding me that I need to check whether or not the directory opened correctly and thank you for the good use of compression.
[reply]

In Section Cool Uses for Perl