| [reply] [d/l] [select] |
Either backticks or open() with a pipe symbol would serve
your purposes. Doing both of them, however, is a Bad Idea.
It ends up trying to execute the
first file in the wildcard pattern ... which presumably
fails, so you get no output.
Fortunately for you, you can replace this whole function
with a single system() call:
system("ls /dir/error* >/tmp/err_list 2>/dev/null");
The "2>/dev/null" makes sure that any error messages from
the 'ls' program won't
be visible to the person running your code.
-- Chip Salzenberg, Free-Floating Agent of Chaos | [reply] [d/l] |
Hi,
There's only one thing really wrong: you shouldn't use backticks with open. Backticks are for when you need all output in a scalar or array, using $list = `ls` or @list = `ls`
I think the "ls $file|" really had to be "ls $mask|".
Calling an external program for simple things like this is considered bad style. Perl has its own ways of listing files: glob, with which you can shell-like globs (* and ? wildcards) and opendir/readdir/closedir.
And another thing about style: most people use ALLCAPS for their file handle names, and all lowercased characters for normal identifiers. These "rules" are described in perlstyle.
sub fill_list {
my ($mask, $file) = @_;
# somewhat more efficient than two shifts
open (LIST, '>', $file) or die "Couldn't open $file: $!";
for (glob($mask)){
print LIST "$_\n";
}
close LIST;
}
Instead of the for-loop, it can be done using a single print statement:
print LIST map("$_\n", glob($mask));
Please note some people consider glob() to be as evil as calling an external program, and some computers have bad libraries, which can cause some trouble when there are file names with spaces in them. (These bad libraries list the two files "File name" and "Hello, World!" as if it were four files: "File", "name", "Hello,", "World!").
2;0 juerd@ouranos:~$ perl -e'undef christmas'
Segmentation fault
2;139 juerd@ouranos:~$
| [reply] [d/l] [select] |
Why don't you read the directory entries with internal Perl functions, instead of relying on some external program (OLDTTGW principle):
Fill_List( '/dir', '^error' );
sub Fill_List {
my $dir = shift;
my $match = shift;
my @found;
opendir DIR, $dir or die "Cannot open directory $dir: $!"\n;
while( defined( my $file = readdir(DIR) )) {
push @found, $file if $file =~ /$match/o;
}
close DIR;
return @found;
}
Note that I am returning the values in an array. It is better to have a routine do one thing, and one thing only. If you want to write the contents out to a file, do that in another routine. Prepare for (obvious) future uses the code could be put to.
Your code has a few style issues that leap out at me:
- You're not using lexical (my) variables, so you're probably not using strict. Until you know why and when it's ok not to use it, you should use strict;
- file handles (list and infile in your example) by convention are always in uppercase;
- if you are not interpolating Perl variables into strings or using meta characters like \n and \t, use 'single quotes' instead of "double quotes";
- and if you are not diddling with subroutine parameter lists, do not use the &func() way of calling function, drop the ampersand. If you don't know what diddling with subroutine parameter lists means, then just avoid using & on subroutine calls.
update: oops, re-reading my code makes me realise this code will only work correctly the first time it is called, due to the /o modifier on the regexp. (It will just keep on returning files that match what the first call was asked to match. The correct method would be to use the qr// operator if the version of Perl supports that. Just put $match = qr/$match/; before the while loop. Otherwise, for earlier versions, one would have to build the loop up in a string, and eval that, which is downright icky.
--g r i n d e r
just another bofh
print@_{sort keys %_},$/if%_=split//,'= & *a?b:e\f/h^h!j+n,o@o;r$s-t%t#u';
| [reply] [d/l] [select] |