Pjoern
Pjoern

Reputation: 339

perl open files with variable in filename

I try to open a list of files in a folder. Eventually I want to insert a HTML-Snippet in it. So far I can open the folder and read the list in an array. That´s fine. But when I try to open the files by using a variable as filename, I get an error message every time ("permission denied"). No matter if I use $_ or other variations of putting the values of list items in variables. I have found similar questions here, but didnt find the solution so far. Here is my code:

use strict;
use warnings;

my ($line);

opendir (FOLDER,"path/to/folder/./") or die "$!";
my @folderlist = readdir(FOLDER);
my @sorted_folderlist = sort @folderlist;
close(FOLDER);

foreach (@sorted_folderlist) {
  my $filename = $_;
  open (READ, ">", "path/to/folder/$filename") or die "$!";
  # do something
  close (READ);
}

What is the mistake here? And how would I open files with using a variable as filename?

Pjoern

Here is my changed code in order to answer 1:

my $dh; 
opendir $dh, "dir/./" or die ... 
my @folderlist = grep { -f "dir/$_" } readdir $dh; 
close $dh; 
my @sorted_folderlist = sort @folderlist; 

foreach my $filename (@sorted_folderlist) { 
  open my $fh, "<", "dir/$filename" or die ... 
  open my $writeto, ">", "new/$filename" or die ... 
  print $writeto "$fh"; 
  close $fh; 
  close $writeto; 
}

Upvotes: 5

Views: 2447

Answers (2)

zdim
zdim

Reputation: 66883

You've got errors and good practices cleared up in tinita's answer.

I'd like to comment on a possible improvement, of building the file list with full paths to start with.

  • Use map on the output of readdir to add directory to each file, then filter that

    my @files = grep { -f } map { "$dir/$_" } readdir $dh;
    

    or, implementing grep's (filtering) functionality in the map block

    my @files = map { my $fp = "$dir/$_"; -f $fp ? $fp : () } readdir $dh;
    

    Here if $fp isn't -f we return an empty list which gets flattened in output, thus vanishing.

  • Use glob, which directly gives you the full path

    use File::Glob ':bsd_glob';
    my @files = grep { -f } glob "$dir/*";
    

    where File::Glob's bsd_glob() overrides glob and handles spaces in names nicely.

Note a difference: the * in glob does not pick entries that start with dot (.). If you want those as well, change it to glob "$dir/{*,.*}". See linked File::Glob docs.


Update   to correct the edit (addition made) to the question, also discussed in comments

Once the full-path filenames have been read into @files

foreach my $file (@files) {
    my $out_file = 'new_' . $file;
    open my $writeto '>', $outfile or die "Can't open $outfile: $!";
    open my $fh,     '<', $file    or die "Can't open $file: $!";
    while (my $line = <$fn>) {
        # process $line and build what need be written to the new file
        my $new_line = ...   
        print $writeto $new_line;
    }
    close $writeto;
    close $fh;
}

(Or, if the filenames aren't full path build the full path in the open call.)

Filehandles get closed when opened again so you need an explicit close only for the last files. Then you can declare my ($fh, $writeto); variables before the loop, use open $fh ... (no my), and close the filehandles after it. But then you have those variables outside of foreach scope.

Documentation:


  The glob "$dir/*" carries the possibility for injection bug, for very particular directory names. While this can come about only wiht rather unusual names it is safer to use escape sequence

my @files = grep { -f } glob "\Q$dir\E/*"

Since this escapes spaces as well the File::Glob with its bsd_glob() isn't needed.

Upvotes: 6

tinita
tinita

Reputation: 4336

You have several issues in your code.

The first, causing the error, probably, is that readdir() will return . and .. also, but these are directories. So you try to write to directory/..

Second, your error message contains $!, which is good, but you don't output the filename. Then you would have seen your mistake.

Third, you call a filehandle you open for writing READ.

Also, you should use lexical filehandles nowadays.

use strict;
use warnings;

opendir my $dh, "dir" or die $!;
# read all files, not directories
my @folderlist = grep { -f "dir/$_" } readdir $dh;
close $dh;
my @sorted_folderlist = sort @folderlist;

foreach my $filename (@sorted_folderlist) {
    open my $fh, ">", "dir/$filename" or die "Could not write to dir/$filename: $!";
    print $fh "foo...\n";
    close $fh;
}

Upvotes: 8

Related Questions