Beefy Boxes and Bandwidth Generously Provided by pair Networks
Keep It Simple, Stupid
 
PerlMonks  

File problems

by coec (Chaplain)
on Dec 19, 2001 at 11:34 UTC ( [id://133043]=perlquestion: print w/replies, xml ) Need Help??

coec has asked for the wisdom of the Perl Monks concerning the following question:

Why doesnt't this work?!?! I want to populate the file /tmp/err_list with the name of each file in /dir that starts with 'error'. I can't see what I'm doing wrong. The file /tmp/err_list gets created but is always empty but there are files the match the pattern.
&Fill_List("/dir/error*", "/tmp/err_list"); sub Fill_List { $mask = shift; $file = shift; open (list, "> $file"); open (infile, "`ls $file|`"); $debug && warn "ls $mask\n"; while (<infile>) { print list $_ . "\n"; } close(infile); close (list); }

Replies are listed 'Best First'.
Re: File problems
by trantor (Chaplain) on Dec 19, 2001 at 12:07 UTC

    Just a few comments to put you on track:

    • start your script with #!/usr/bin/perl -w (or your path)
    • always use strict at the beginning of your program, in this way you'll see you need my $debug and my in front of $mask and $file as well
    • spell filehandles in CAPITAL LETTERS, it's not just a convention, future reserved words may clash with your filehandles otherwise
    • since you're using open, there's no need for backticks inside double quotes. Better yet, get rid of that nasty open using glob in your while loop. Not only this improves your script's security, but it doesn't need to call an external program anymore

    I hope this helps!

    -- TMTOWTDI

Two rights make a wrong
by chip (Curate) on Dec 19, 2001 at 11:54 UTC
    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

Re: File problems
by Juerd (Abbot) on Dec 19, 2001 at 12:37 UTC
    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:~$

Re: File problems
by grinder (Bishop) on Dec 19, 2001 at 13:18 UTC
    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';

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://133043]
Approved by root
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others drinking their drinks and smoking their pipes about the Monastery: (4)
As of 2024-04-18 04:10 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found