Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl Monk, Perl Meditation
 
PerlMonks  

Safe way to open files

by L0rdPhi1 (Sexton)
on Jun 15, 2002 at 13:44 UTC ( #174837=perlquestion: print w/replies, xml ) Need Help??

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

I'm developing a script and want to create a sub that'll open files safely at all times. Below I've posted the sub I'm currently using which seems to be working okay except once in a blue moon a file will go to zero bytes totally randomly with no explanation. I've narrowed the problem down to something in this sub but I can't seem to find what is exactly causing it... so I've come to seek help :) Also, in the below snippet $flock_enabled will either equal 1 or 0 depending on if flock is enabled on the users system or not.

This sub would be used like: my $filehandel = safe_open("some.file"); and the sub fatal_error has nothing more then this in it: my $error = shift;print "content-type: text/html\n\n$error";exit;
sub safe_open { my $fn = shift; my $fh = do { local *FH }; my ($mode, $result); if ($fn =~ /^>[^>]/) { $mode = 1; } elsif ($fn =~ /^>>/) { $mode = 2; } if ($mode == 1 and $flock_enabled) { # Write mode with flock. $do_trun = 1; $result = sysopen($fh, $fn, O_CREAT | O_WRONLY); } elsif ($mode == 1) { # Write mode without flock. $result = open($fh, ">$fn"); } elsif ($mode == 2) { # Append mode. $result = open($fh, ">>$fn"); } elsif (!$mode) { $result = open($fh, $fn); } unless ($result) { # Open failed. fatal_error("Couldn't open $fn."); } flock($fh, 2) if $flock_enabled; truncate($fh, 0) or fatal_error("Could not truncate file $fn.") if + $do_trun; return $fh; }
Thanks!

Edit kudra, 2002-06-15 Added READMORE

Replies are listed 'Best First'.
Re: Safe way to open files
by vladb (Vicar) on Jun 15, 2002 at 13:58 UTC
    Althought, I don't have an immediate direct answer to your question, I felt that in the mean time I could suggest a few improvements to your subroutine. I'll update this node if I find a suitable explanation for your problem at hand, though.

    Normally I feel it's a good idea to separate the code that reports something from the code that simply does something. Therefore, as an improvement to your sub, I would move the call the fatal_error() method (which simply prints an error string to the client) outside of your subroutine and let the caller script handle the outcome of the safe_open() method. You could also look into making your method die with a custom error message, which could be picked up by a caller script using the try { }; if ($@) { ... } construct. So, the sub would look similar to this:
    # MAIN my $fn = shift; try { safe_open($fn); }; if ($@) { # here, print fatal error fatal_error($@); } # SUBS sub safe_open { my $fn = shift; my $fh = do { local *FH }; my ($mode, $result); # reset the do_trun variable. # and also, make it local to this sub.. why keep it global? ;) my $do_trun; if ($fn =~ /^>[^>]/) { $mode = 1; } elsif ($fn =~ /^>>/) { $mode = 2; } if ($mode == 1 and $flock_enabled) { # Write mode with flock. $do_trun = 1; sysopen($fh, $fn, O_CREAT | O_WRONLY) or die "Failed to open '$fn' in write mode with flock."; } elsif ($mode == 1) { # Write mode without flock. open($fh, ">$fn") or die "Failed to open '$fn' for writing (no flock)."; } elsif ($mode == 2) { # Append mode. open($fh, ">>$fn") or die "Failed to open '$fn' for appending."; } elsif (!$mode) { open($fh, $fn); or die "Failed to open '$fn'."; } if ($flock_enabled) { flock($fh, 2) or die "Failed to flock '$fn'."; } if ($do_trun) { truncate($fh, 0) or die "Could not truncate file $fn."; } return $fh; }


    Update: Err.. I guess this is a classical example of how things might go wrong when you resort to using global variables. From the code, I can't quite see where you receive/get/set/modify the $do_trun variable. It might be that you still have it set to some positive (true) value on consequent calls to the method, which causes your file to be truncated. I guess, you'll simply have to reset the $do_trun variable to 0 every time inside the actual sub to eliminate the 'once in a blue moon' phenomena. ;-)

    _____________________
    # Under Construction
Re: Safe way to open files
by danger (Priest) on Jun 15, 2002 at 17:42 UTC

    Here's an approach that reduces the number of file-opening calls you need to manage --- determine what flags you want on the file handle and use sysopen once. In the following modes 1 and 2 act like > and >> respectively, else the file is opened read-only. This can be altered --- if you'd rather fail on attempting to append to a non-existing file, remove the O_CREAT flag from mode 2 for example.

    use Fcntl qw(:DEFAULT :flock); # assume $flock_enabled set accordingly sub safe_open { my $fn = shift; $fn =~ s/^(>>?)//; $mode = length($1); my $flags = $mode == 1 ? O_CREAT | O_WRONLY | O_TRUNC : $mode == 2 ? O_CREAT | O_WRONLY | O_APPEND : O_RDONLY; sysopen(my $fh, $fn, $flags) || die "Can't open $fn: $!"; flock($fh, LOCK_EX) || die "Can't flock $fn: $!" if $flock_enabled +; return $fh; }
      I'm now using the below. Anyone see any problems? Should I still use truncate($fh, 0); after seek or does the sysopen take care of that?
      sub safe_open { my $fn = shift; $fn =~ s/^(>>?)//; $mode = length($1); my $flags = $mode == 1 ? O_CREAT | O_WRONLY | O_TRUNC : $mode == 2 ? O_CREAT | O_WRONLY | O_APPEND : O_RDONLY; sysopen(my $fh, $fn, $flags) or die "Can't open $fn: $!"; if ($flock_enabled) { flock($fh, LOCK_EX) or die "Can't flock $fn: $!"; } seek $fh, 0, 0; return $fh; }
        No. You do not want an unconditional seek in the routine. If it was opened with O_TRUNC or O_RDONLY it isn't needed (neither is truncate), if it was opened with O_APPEND then you definitely do not want to seek nor truncate.
Re: Safe way to open files
by Aristotle (Chancellor) on Jun 15, 2002 at 15:33 UTC

    Update: danger++ made some excellent improvements.

    As has already been hinted, at put a my $do_trun; at the top of the routine. use strict and warnings to avoid such problems in the future.

    Also, I want to point out something else: if ($fn =~ /^>[^>]/) obviously expects the string to already have the angle brackets embedded. But then you go and add some more in $result = open($fh, ">$fn"); and $result = open($fh, ">>$fn");

    And while not detrimental, they shouldn't be in sysopen($fh, $fn, O_CREAT | O_WRONLY); either. Together with the also already suggested removal of your fatal_error calls, I suggest the following:
    sub safe_open { my $fn = shift; # you need not initialize $fh anymore in Perl >= 5.6 my ($fh, $mode, $result, $do_trun); $fn =~ s/^(>>?)//; $mode = length($1); if ($mode == 1) { # write mode if($flock_enabled) { $do_trun = 1; $result = sysopen($fh, $fn, O_CREAT | O_WRONLY); } else { $result = open($fh, ">$fn"); } } elsif ($mode == 2) { # append mode $result = open($fh, ">>$fn"); } else { # read mode $result = open($fh, $fn); } $result or die "Couldn't open $fn: $!"; flock($fh, 2) or die "Couldn't flock $fn: $!" if $flock_enabled; truncate($fh, 0) or die "Could not truncate file $fn: $!" if $do_t +run; return $fh; }
    ____________
    Makeshifts last the longest.
Re: Safe way to open files
by L0rdPhi1 (Sexton) on Jun 15, 2002 at 14:03 UTC
    Thanks, That's a good idea :) I just cannot figure out the zero bytes bug... I've managed to stump quite a few people with it, by the way. But I think it has something to do with sysopen and truncate but I can't be sure.


    Update:

    I totally recoded the old sysopen part but I'm not sure if what I did will work...
    sub safe_open { my $fn = shift; my $fh = do { local *FH }; my ($mode, $result); if ($fn =~ /^>[^>]/) { $mode = 1; } elsif ($fn =~ /^>>/) { $mode = 2; } else { $mode = 0; } if ($mode == 1 and $flock_enabled) { # Write mode with flock. $truncate_please = 1; my $fhtemp = do { local *FHT }; $result = open( $fhtemp, "+>>$fn" ); if ($result) { $result = open($filehandle, ">&$fhtemp" ); close($fhtemp); } } elsif ($mode == 1) { # Write mode without flock. $result = open($fh, ">$fn"); } elsif ($mode == 2) { # Append mode. $result = open($fh, ">>$fn"); } elsif ($mode == 0) { $result = open($fh, $fn); } unless ($result) { # Open failed. fatal_error("Couldn't open $fn."); } flock($fh, 2) if $flock_enabled; if ($truncate_please) { truncate($fh, 0) or fatal_error("Could not truncate file $fn." +); } return $fh; }
Re: Safe way to open files
by belg4mit (Prior) on Jun 17, 2002 at 02:43 UTC
    Depending on your definition of safe you should be using 3-arg open instead of 2-arg. You could also make your sub take parameters like 3-arg, and save yourself the string bashing ;-)

    --
    perl -pew "s/\b;([mnst])/'$1/g"

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others exploiting the Monastery: (8)
As of 2021-05-12 11:06 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    Perl 7 will be out ...





    Results (126 votes). Check out past polls.

    Notices?