Filehandle in subroutine in use VRML.pm

[smittypaddler]

I have this code in my DrillPressTable.pl file:

 #!/usr/bin/perl -w
 use strict 'vars';
 use VRML;
 ...
 my $Dir="vrmlstuff";
 my $Subdir="DrillPressTable";
 my $LumberFile="$Dir/$Subdir/lumber.wrl";
 &mystart($LumberFile,LF);
 ...
 sub mystart {
   my($file,$fh)=@_;
   open($fh,"> $file")
    || die("ERROR: Unable to open $file for output");
   my @lines=split(/\n/,<<END);
#VRML V2.0 utf8
NavigationInfo { headlight TRUE }
DirectionalLight { # First child
 direction 0 0 -1 # Light illuminating the scene
}
END
   my $i=0;
   if($i==0) {
     foreach my $line (@lines) {
       print $fh "$line\n";
     }
   } elsif($i==1) {
      &myprint(\@lines,$fh);
   } elsif($i==2) {
      &printout(\@lines,$fh);
   }
 } # end mystart
 ...
 sub myprint(
   my($lines,$fh)=@_;
   foreach my $line (@{$lines}) {
     print $fh "$line\n";
   }
 } # End myprint;

file VRML.pm contains this code:
 package VRML;
 BEGIN {
   @EXPORT = qw(... &printout ...);
 }
 use strict 'vars';
 ...
 sub printout {
   my($lines,$fh)=@_;
   $fh=STDOUT unless($fh);
   foreach my $line (@{$lines}) {
     print $fh "$line\n";
   }
} # End printout;

If I set $i=0 or 1 in subroutine mystart it works perfectly, and the proper lines are written to lumber.wrl. But if I set $i=2 I get this error message: print() on unopened filehandle LF at .../VRML.pm line 2886 Line 2886 in VRML.pm is in printout; the line is:

print $fh "$line\n";

How do I declare $fh in subroutine printout so it recognizes the intended filehandle?

[haukex]

Thank you for providing enough code to reproduce your issue!

The problem is that because you're not using strict "refs" (Update: and strict "subs"), there is some funky symbolic reference stuff going on here. It's unclear to me why you even need to pass the parameter LF in &mystart($LumberFile,LF);, but AFAICT what is going on here is that that call is actually being interpreted as &mystart($LumberFile,"LF");, and then inside VRML.pm there is no filehandle named LF because that was opened in the main package.

The best thing to do is to rewrite your code so that it works under strict and warnings. The minimum changes I would recommend is to change the definition and call of mystart as follows:

mystart($LumberFile);
sub mystart {
    my ($file) = @_;
    open( my $fh, "> $file" ) or die ...

If you need the $fh parameter to mystart, you need to show us more context as to how it gets used for us to make a recommendation for a useful replacement (Update: Re^3: Filehandle in subroutine in use VRML.pm).

However, I see several other older code styles in your sample code, for example using the -w shebang switch (What's wrong with -w and $^W), using the old ampersand-style sub calls (perlsub), and the two-argument open ("open" Best Practices). So here's your sample code updated to modern Perl, I strongly suggest you adopt this style for all your code:

Read more... code (2 kB)

[smittypaddler]

Thanks for the quick response. It will take me awhile to formulate why myprint() even exists, and what my ultimate goal is, so please give me some time. Regarding the archaic syntax in my perl programs, I'm 80 years old, and've been programming since 1965, so if we were conversing you'd probably hear some stuff like "Awesome," "Beet Feet" and "Bug out" from me. Anyway, I'll come back to this, and thanks again for the response.

[haukex]

Regarding the archaic syntax in my perl programs, I'm 80 years old, and've been programming since 1965, so if we were conversing you'd probably hear some stuff like "Awesome," "Beet Feet" and "Bug out" from me.

Sure, I understand, and a few of the edits I made are indeed purely stylistic (for is equivalent to foreach, dropping the parens on function calls, or running the code through perltidy), but at least the changes I named in my node above have become best practices for good reason, as explained in the links. And Use strict and warnings is quite important - your question is a good example of that and the potential of symbolic references to cause confusion and hard to diagnose bugs.

[Marshall]

I didn't try to run your entire code.
I am replying to your question: "How do I declare $fh in subroutine printout so it recognizes the intended filehandle?".

This is not right: $fh=STDOUT unless($fh); Try this instead:

use strict; # always enable all strictures
use warnings;

my @lines = ("asdf", "xyz");

printout (\@lines);
#
# Default to STDOUT if no file handle specified
#
sub printout {
   my($line_ref,$fh)=@_;
   $fh= *STDOUT unless ($fh); # *STDOUT is a typeglob
   foreach my $line (@{$line_ref}) {
     print $fh "$line\n";
   }
} # End printout;

[smittypaddler]

In the past my vrml scripts have always generated one set of VRML statements written to STDOUT on a given run, but in this latest script I wanted to create two wrl files in DrillPressTable.pl:

 my $LumberFile="$Dir/$Subdir/lumber.wrl";
 my $DrillPressTableFile="$Dir/$Subdir/$Subdir.wrl";
 &mystart($LumberFile,LF);
 &buildlumber(\%Fs,LF,$Ppi);
 &mystart($DrillPressTableFile,DP);
 &buildtable(\%Fs,DP,$Ppi);

The original printout subroutine in VRML.pm looked like this, which I couldn't now use in DrillPressTableFile.pl because it had no provision for directing output to a filehandle other that STDOUT:

<code> sub printout { my($lines)=@_; foreach my $line (@{$lines}) { print "$line\n"; } } # End printout; </code)

The suggestion by Mr. Marshall didn't work, because the problem occurs when $fh at entry to printout is defined. Setting $fh to STDOUT is to have a default, and perhaps that would've been a problem if I'd gotten past the original problem of passing a filehandle to printout.

The myprint subroutine was to be only temporary until I figured out how to change printout to accept a filehandle, which brings me to my original question: How do I declare $fh in subroutine printout so it recognizes the intended filehandle?

[haukex]

which brings me to my original question: How do I declare $fh in subroutine printout so it recognizes the intended filehandle?

The problem is not in sub printout, it's here:

&mystart($LumberFile,LF);
&buildlumber(\%Fs,LF,$Ppi);

This gives us a little more context as to what's going on. You apparently want to use the "filehandles" LF and DP like variables and pass them as arguments to the subs to use. But because of the missing strict, the LF is being taken as a bareword, that is, even though it looks like a filehandle, it's actually the string "LF" (B::Deparse on the sample code from the root node shows this). Then AFAICT this is being used as a symbolic reference by open and print, and this resolves it relative to the current package. When everything is in the main package, it works, but sub printout is in a different package, so it fails.

Here's one possible solution that requires only very small changes to your code and even works under strict once you get around to using that. The do block basically generates a new filehandle (see also my node here for alternatives). This will actually give you a filehandle stored in a variable that you can pass around.

my $LF = \do { local *LF; *LF };
&mystart($LumberFile,$LF);
&buildlumber(\%Fs,$LF,$Ppi);

I do still feel compelled to say that this is still just triage, though. The real issue is the missing strict compliance.

[Marshall]

Hi smittypaddler,

Again, you should use all strictures. use strict 'vars'; limits strict checking. use strict; is what you want.

The code change that I gave you for printout() is correct. printout()won't work without that change. Past that, the problem is how you call printout().

You cannot pass a simple bareword file handle like STDOUT as a parameter. Bareword filehandles are global in scope and don't need to be passed as parameters because they are known everywhere, in all subroutines. If you want to pass filehandles as a parameter, you should use lexically scoped filehandles (using "my") which can be passed as parameters. Also as a style rule, whatever level of the program opens a file, that level should be responsible for closing it.
In your case, I would move the opening of the Lumber File up to the main program level. There is no need to ever create the "bareword filehandle, LH". Start off by creating a lexical filehandle to begin with and you don't have to worry about "translating" bareword to normal variable.

my $LumberFile="$Dir/$Subdir/lumber.wrl";
open my $Lumber_fh, '<', $LumberFile  or warn "Ooops Cannot Open Lumbe
+r File: $LumberFile !! \n Proceeding using STDOUT";
my $out_fh = *STDOUT unless (defined $Lumber_fh);
...
As a style point, I add "_fh" ("filehandle") as a suffix to remind
myself that this variable is a filehandle and not some 
integer or other thing.

The "*" in front of STDOUT is critical!  
This allows the "translation" of the bareword filehandle STDOUT
into the lexical program variable "my $out_fh".
Without that, Perl figures that STDOUT is a string.
Now you can pass $out_fh as a parameter to other subs or use
it yourself, e.g.
print $out_fh "something";

printout() is not needed. A single line suffices and is more clear.
print $out_fh "$_\n" for @lines;

Update:
The error processing above is rather crude and I don't recommend it. In general, if there is a directory path that is supposed to be the "output file" and you can't open it, you should "die" and await changes to that filename/permissions. Normally you would not keep going and "waste" what might potentially be a significant computation effort. I would treat a null string $LumberFile name and non-null string $LumberFile differently.

You might find amusing: A long time ago, when I was working with IBM mainframes, we had a job that ran for 5 days straight. That did not alarm us. We expected that. The unexpected part came when program abended due to a simple JCL (Job Control Language) error before writing the result to a magnetic tape. We had about 16 tape drives on that machine and it would have been completely fine for the program to "open" one of the drives for 5 days and hang on to it. The coding error that involved acquiring a tape drive for output wasn't apparent until after 5 days of number crunching! We lost 5 days of computing, the cost of which was not insignificant. So, in general I would open the output file before I had calculated some result to output to it.

[bliako]

I understand that you want to pass filehandles to functions with the added feature that if user specified an undefined handle, said function and the rest of the program should use default STDOUT.

Firstly, try and migrate from bareword filehandles to lexical ones, except when you use the standard ones: STDERR, STDIN, STDOUT. So avoid using open FH, '>', 'xyz'; in favour of my $fh; open $fh, '>', 'xyz';

printout() below accepts a reference to a variable containing the filehandle. If contents of ref are not defined then it sets them to STDOUT and then proceeds in printing to the filehandle by dereferencing the ref (using {$$fhref} the curlies are used for disambiguating the syntax). Setting the fhref to STDOUT affects the caller since it is a reference to a variable of the caller.

my $fh = undef;
print "1.fh is now $fh\n";
printout("some lines", \$fh);     
print "2.fh is now $fh\n";
printout("some lines", \$fh);
print "3.fh is now $fh\n";
$fh = undef; # <<<< very important, else redirects STDOUT to xxx
open $fh, '>', 'xxx';
printout("some lines", \$fh);
print "4.fh is now $fh\n";
close $fh;

sub printout {
  my ($lines, $fhref) = @_;
  $$fhref = *STDOUT unless $$fhref;
  print {$$fhref} $lines;
}

There is some serious potential bug with above code. In open $fh, '>', 'xxx';, if $fh is set to STDOUT (and not undef as is the usual use-case) then it redirects STDOUT to 'xxx'! This and because I prefer that the one who opens the filehandle to close it as well, I would not use this idea of printout() setting the filehanlde to STDOUT which can then be inherited to all subsequent calls by the caller. I find it an unecessary complication. But printout() printing to STDOUT if no filehandle was given, is fine logic to me. So:

my $fh = undef;
printout('some lines1', $fh);
open $fh, '>', 'xxx';
printout('some lines2', $fh);
close $fh;

sub printout {
  my ($lines, $_fh) = @_;
  my $fh = $_fh ? $_fh : *STDOUT;
  print $fh $lines;
}

bw, bliako

[smittypaddler]

If I've understood Mr Bliako's message correctly, it doesn't work. The open in DrillPressTable.pl and the call to printout are the same, and the error message begins "print() on unopened filehandle LF...", and printout is changed according to his message:

sub printout {
  my ($lines, $_fh) = @_;
  my $fh = $_fh ? $_fh : *STDOUT;
  foreach my $line (@{$lines}) {
    print $fh "$line\n";
  }
} # End printout;

[haukex]

... despite my various solutions and workarounds from my nodes here and here being much better than the following, if you're hell-bent on only modifying sub printout, here's enough rope to shoot yourself in the foot: (Update: Note the following only works because of the absence of strict...)

sub printout {
  my ($lines, $_fh) = @_;
  my $fh = $_fh ? caller.'::'.$_fh : *STDOUT;
  foreach my $line (@{$lines}) {
    print $fh "$line\n";
  }
} # End printout;

Though you may need to replace caller with the string "main" if printout gets called from another package. /shudder

[bliako]

I understand that you want to pass filehandles to functions with the added feature that if user specified ...

Alas it seems I did not understand exactly. In my code above I assume that you will open a file using open my $fh, ... and that you pass around that filehandle and not a literal string/bareword representing also a filehandle, which gets complicated, having to specify also package names. So, in the code I provided the part open $fh, '>', 'xxx'; is essential. The following works for me:

{
package main;
my $fh = undef;
VRML::printout('some lines1', $fh);
open $fh, '>', 'xxx';
VRML::printout('some lines2', $fh);
close $fh;
}

{
package VRML;
sub printout {
  my ($lines, $_fh) = @_;
  my $fh = $_fh ? $_fh : *STDOUT;
  print $fh $lines;
}
}

bw, bliako

[BillKSmith]

The documentation for open states:

If FILEHANDLE is an undefined scalar variable (or array or hash element), a new filehandle is autovivified, meaning that the variable is assigned a reference to a newly allocated anonymous filehandle. Otherwise if FILEHANDLE is an expression, its value is the real filehandle. (This is considered a symbolic reference, so use strict "refs" should not be in effect.)

You initialize $fh with LF. Under strict, all three options produce the expected error:

Can't use string ("LF") as a symbol ref while "strict refs" in use ...

The real question is why do options 0 and 1 appear to work as intended without strict. I have no idea.

The minimal (not recommended) 'fix' is to remove LF from the call to mystart. This leaves $fh uninitalized.

#&mystart($LumberFile,LF);
 &mystart($LumberFile);

[smittypaddler]

None of the suggestions have worked, but I found a solution. In my VRML.pm file, in any subroutine printing output, I added a myprint argument and where the subroutine originally invoked &printout, I changed it to invoke \&myprint, where @lines is an array of lines to be printed:

sub egg { # prints a 3d egg.
 my %args=(myprint => \&printout, xaxis => 10, ...);
 # Produces lines of wrl output in @lines ...
 $myprint=$args{myprint};
 &{$myprint}(\@lines);
}

In DrillPressTable.pl, which uses VRML.pm, I added a global $Fh, and changed the invocation of mystart, and its code, and myprint's code as well:

my $Fh; # w/b set in mystart.
&mystart($LumberFile,LF);
sub mystart {
 my($file,$fh)=@_;
 open($fh,"> $file")
  || die("ERROR: Unable to open $file for output");
 $Fh=$fh;
 my @lines=split(/\n/,<<END);
#VRML V2.0 utf8
NavigationInfo { headlight TRUE }
...
END
 &myprint(\@lines);
}

sub myprint {
  my($lines)=@_;
  foreach my $line (@{$lines}) {
    print $Fh "$line\n";
  }
}

So now in DrillPressTable.pl I can direct lines of output to multiple file handles, and I don't need to change any of the 70-odd perl programs that use my VRML.pm. They'll just take the default of &printout, which prints to STDOUT, just like before.

[haukex]

None of the suggestions have worked

All five (!) of the variations of solutions I posted in this thread work correctly when applied to the sample code you posted. This means that either you did not apply the suggestions correctly, or that your sample code was not representative of your actual code. In either case, see Short, Self-Contained, Correct Example and I know what I mean. Why don't you?

[BillKSmith]

I am pleased to hear that you solved your problem yourself. Are you aware that this new solution is still not compliant with strict refs? You have already demonstrated the problem with this. Minor changes can break your code in ways that are very hard to debug. Not a good idea for a widely used module! At the very least, I would recommend that you specify use strict for the whole program. Specify no strict refs in the smallest possible scope.

It now sounds like your real problem is how to change the default output of your module. Look at the library function select. It may do exactly what you want.