Perlcritic - Two argument "open" error

Question

I have a script and I am trying to elimate bad practices using perlcritic.

One line I have is as follows:

open(my($FREESPCHK), $cmdline ) || &zdie($MSG_PASSTHRU,"Error checking free space of file system.");

This gives this error: Two-argument "open" used at line xxx, column x. See page 207 of PBP. (Severity: 5)

Any ideas on how to fix it?

Answer 1

If you use the --verbose 11 flag, you'll get a far more detailed explanation of the error. In this case, the error you get is looks like this:

Two-argument "open" used at line 6, near 'open FILE, 'somefile';'.
InputOutput::ProhibitTwoArgOpen (Severity: 5)

The three-argument form of `open' (introduced in Perl 5.6) prevents subtle bugs that occur when the filename starts with funny characters like '>' or '<'. The IO::File module provides a nice object-oriented interface to filehandles, which I think is more elegant anyway.

 open( $fh, '>output.txt' );          # not ok
 open( $fh, q{>}, 'output.txt' );     # ok

 use IO::File;
 my $fh = IO::File->new( 'output.txt', q{>} ); # even better!

It's also more explicitly clear to define the input mode of the file, as in the difference between these two:

  open( $fh, 'foo.txt' );       # BAD: Reader must think what default mode is
  open( $fh, '<', 'foo.txt' );  # GOOD: Reader can see open mode

This policy will not complain if the file explicitly states that it is compatible with a version of perl prior to 5.6 via an include statement, e.g. by having `require 5.005' in it.

I found this by reading the perlcritic documentation.

Answer 2

To make Perl Critic shut up, but do no real good at all, just modify the code to:

open(my $PIPE_FROM_FREESPCHK, "-|", $cmdline)
    || zdie($MSG_PASSTHRU, "Error checking free space of file system.");

Note however that this is absolutely no better in any regard whatsoever from the far more obvious:

open(my $PIPE_FROM_FREESPCHK, "$cmdline |")
    || zdie($MSG_PASSTHRU, "Error checking free space of file system.");

Because you are not separating out your tokens for calling exec directly. That would look more like this:

open(my $PIPE_FROM_FREESPCHK, "-|", $cmd_name, @cmd_args)
    || zdie($MSG_PASSTHRU, "Error checking free space of file system.");

The question is whether you are running a shell command or just exec’ing something. If your free check is something like df . 2>/dev/null | awk ...., then you need the full shell. If it is just df, then you don’t.