Re^2: list lines not found in config (while+if)

Note kennethk's comments regarding the two parameter open in his reply to the OP!

What kennethk forgot to mention was that you should use lexical file handles too.

open ... $filename || die makes for an unhappy life. || binds to $filename, not to the result of open as you may be hoping. $filename is true for all likely values so the die will never fire, regardless of what the result of the open is! Use open ... $filename or die instead. It's often helpful in the die to show the error message associated with the open failure using $OS_ERROR ($!).

Making those changes, removing extraneous () and minor adjusting of white space produces the following (untested) code:

#!/usr/bin/perl
use strict;
use warnings;

die "Usage fileReference File2Check >outfile" if @ARGV != 2;

my ($fileRef, $file2Check) = @ARGV;

open my $fileREFIn, '<', $fileRef or die "unable to open $fileRef: $!"
+;
open my $fileCHKIn, '<', $file2Check or die "unable to open $file2Chec
+k: $!";

my %seen = map { chomp; $_ => 1 } <$fileREFIn>;

print grep {! $seen{(split /,/, $_)[0]}} <$fileCHKIn>;
[download]

True laziness is hard work

Comment on Re^2: list lines not found in config (while+if) Select or Download Code

Replies are listed 'Best First'.
Re^3: list lines not found in config (while+if) by Marshall (Canon) on Apr 08, 2009 at 02:48 UTC
Great points Grandfather! I think there can be some legitimate differences of opinions on these things. First on the subject of: open ... $filename \|\| die makes for an unhappy life. Out of force of habit, I use more parens so that this sort of thing is not a problem. open (...) or die "..." is the same as open (...) \|\| die "...". Which is NOT the same as open ... \|\| die "...". So you are correct that there is a potential problem here! I recommend to always use parens to make things clear. Especially on calls to the O/S! Use of ?! is a grey area here. Probably more important is one thing that we didn't talk about: the significance of \n in a "die". If there is no \n in the "die text" Perl will report the text message and the program line number. If I get called on the phone by a user with a fatal error, that is very useful information to me! If there is a \n in the die, I won't get the line number! Whether or not there is a ?! error is of much less importance. So if user types: C:\PROJECTS\PerlMonks>test.pl f3 f2.txt ERRORMSG: unable to open f3 at C:\PROJECTS\PerlMonks\test.pl line 6. I know what happened. If we have the ?! also, then we get: ERRORMSG: unable to open f3 No such file or directory at C:\PROJECTS\PerlMonks\test.pl line 6. That in this case is pretty much the same thing. Most important is a good error message and leaving off the \n in the die statement. BUT, I would agree to stick that $! thing in there! I usually do it, but in this case sometimes we overburden new folks with the 2nd level of detail that isn't so important at the time. I would like to be educated re: security holes. For these very short 10 line things, I don't see a problem with the way that I opened the 2 read-only files. Stuff that comes from cgi scripts etc is way different. There isn't a problem here, but I suspect that the answer will be "hey, there could be a problem in a another situation...".	[reply] [d/l]
Re^4: list lines not found in config (while+if) by GrandFather (Saint) on Apr 08, 2009 at 03:28 UTC
or die works regardless of there being parentheses or not, it's the same number of characters and it implies flow control so there really isn't a good reason to use \|\|. Using too many extra (for some value of too many) parentheses makes code harder to read, write end edit correctly because of the burden of ensuring the parentheses are matched correctly. Understanding operator precedence for at least the common operators is a prerequisite for using a programming language effectively and clearly. I have almost the opposite view on the use of \n in a die in the context of a file open failure, but that's because most often I'm writing tools that perform few opens and almost always very early on in processing. In those cases the file name and failure mode are the important information and the actual line number is noise, especially for users. The way you write 10 line code fragments should be very little different than the way you write larger fragments. Getting into the habit of writing robust code will save you time in the long run. One security issue that generally arises with the 2 parameter open is that people who use it generally also omit the '<' which allows a bad person to supply a '>' instead with fairly obvious results. A more subtle and much nastier way to subvert a two parameter open is to prefix or append '\|' to the 'file name'. The file name then turns into a command line which opens all sorts of interesting possibilities to to nasty minded. The burden of using the three parameter version of open compared with the two parameter version is so slight that there is almost no reason not to use it. True laziness is hard work	[reply]
Re^5: list lines not found in config (while+if) by Marshall (Canon) on Apr 08, 2009 at 04:43 UTC
Fair enough, some difference of opinion, but appears to me pretty similar. Too many parens is as bad as too few. I use parens on most sub calls with some Perl bareword exceptions. print, join and some of those things... I always use paren's when calling my own subs. Actually as you can see, we have the same philosophy of coding: 1. check args...first thing... 2. open files..very early in processing.. 3. do processing For user errors, I find for my users (who aren't that sophisticated), my error message is enough in the "die", "can't find APP X's config file $path/.ini" or similar is enough. If they can't figure it out from that, then I want to know the line number. The $! info is noise to them. That is "in-between info" and I put it there for more sophisticated users. So I do put it in there. On open's, well fair enough. 3 parms is "better", but I can't say that with authenticated users 2 parms like I did is bad. But few extra characters is no problem.	[reply]