5

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; 
}
10
  • 2
    I've edited both of your code examples to add indentation. You're welcome, but please do it yourself in the future. Proper indentation is an important tool to help people understand your code. If you're asking a large group of strangers to read and understand your code, then surely it is only polite to make that as simple as possible for them. Commented Dec 16, 2017 at 11:23
  • 1
    Hmm. It's very kind that you edited your question and now show your working code after you applied @tinita's proposal. I do understand that you want to share that and tell us "look how I made it working". But now it's no longer a question. It already includes an answer. People will try to figure out what's wrong with your 2nd code snippet and find … nothing. /// I'm not sure how to handle this. Anybody an idea? @Dave? Commented Dec 16, 2017 at 19:41
  • Uh, I edited my question because I actually thought it would be polite to show that I appreciated the answer. I can delete the second code example, if its reasonable. @DaveCross: Sure, I´ll look at it. Commented Dec 16, 2017 at 20:04
  • 1
    @PerlDuck: I think the question is still as clear as it was. And there are still obvious problems in the second code example - for example print $writeto "$fh" is very strange :-) Commented Dec 16, 2017 at 20:31
  • @Pjoern You added print $writeto "$fh";. That prints to $writeto alright, but what it prints is the filehandle $fh itself (evaluating it under "" is not needed and doesn't change anything.); so it print something like GLOB(0x1820a68). Instead, read from that input, process the line, and print that to output while(my $line = <$fh>) { process-$line; print $writeto $line; } Commented Dec 16, 2017 at 20:42

2 Answers 2

8

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;
}
Sign up to request clarification or add additional context in comments.

1 Comment

@tinita: Thanks for your answer, with your example I think I can get my code working - although I didn´t understand every detail. My usage of readdir() is the crucial error I think. Thanks for your tips to modern code, too :). I´ve posted my code so far in my question (@Dave Cross). Im working on it.
6

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.

1 Comment

glob is the preferred method for scanning a directory; File::Find is preferred for scanning sub-directories too.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.