Skip to content

Three-argument open used #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Three-argument open used #76

wants to merge 4 commits into from

Conversation

@IvanLudvig
Copy link

@IvanLudvig IvanLudvig commented Oct 20, 2019 •
edited
Loading

Replaced two-argument open calls in 13 files to three-argument calls. #75

briandfoy
briandfoy requested changes Oct 20, 2019
View reviewed changes
Copy link
Owner

@briandfoy briandfoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IT looks like you might have changed these with a script. That sort of approach often causes problems. Take a look at each change individually to see that the change makes sense.

bin/awk Outdated

open(TMPIN, "> " . ($tmpin = "a2pin.$$")) ||
open(TMPIN, "> " . ($tmpin = "/tmp/a2pin.$$")) ||
open(TMPIN, '<', "> " . ($tmpin = "a2pin.$$")) ||
Copy link
Owner

@briandfoy briandfoy Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look suspect. I think you've done the wrong thing here.

bin/awk Outdated
$program = do { local $/; <TMPOUT> };
close(TMPOUT) || die "can't close $tmpout: $!";
open(STDOUT, ">&SAVE_OUT") || die "can't restore stdout: $!";
open(STDOUT, '<', ">&SAVE_OUT") || die "can't restore stdout: $!";
Copy link
Owner

@briandfoy briandfoy Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check this one

bin/awk Outdated
close(TMPIN) || die "can't close $tmpin: $!";

open(STDOUT, ">&TMPOUT") || die "can't dup to $tmpout: $!";
open(STDOUT, '<', ">&TMPOUT") || die "can't dup to $tmpout: $!";
Copy link
Owner

@briandfoy briandfoy Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check this one

bin/awk Outdated

open(TMPOUT,"+> " . ($tmpout = "a2pout.$$")) ||
open(TMPOUT,"+> " . ($tmpout = "/tmp/a2pout.$$")) ||
open(TMPOUT,'<', "+> " . ($tmpout = "a2pout.$$")) ||
Copy link
Owner

@briandfoy briandfoy Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check these

bin/awk Outdated
open(TMPIN, "> " . ($tmpin = "a2pin.$$")) ||
open(TMPIN, "> " . ($tmpin = "/tmp/a2pin.$$")) ||
open(TMPIN, '<', "> " . ($tmpin = "a2pin.$$")) ||
open(TMPIN, '<', "> " . ($tmpin = "/tmp/a2pin.$$")) ||
Copy link
Owner

@briandfoy briandfoy Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check these

bin/split Outdated
# MS-DOS: $curname = "$prefix." . $curext;
$curname = $prefix . $curext;
open (FH, ">$curname") or die "$me: Can't open $curname: $!\n";
open (FH, '<', ">$curname") or die "$me: Can't open $curname: $!\n";
Copy link
Owner

@briandfoy briandfoy Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check this

bin/split Outdated
my $prefix = ($ARGV[1] ? "$ARGV[1]" : "x");

open (INFILE, "$infile") || die "$me: Can't open $infile: $!\n";
open (INFILE, '<', "$infile") || die "$me: Can't open $infile: $!\n";
Copy link
Owner

@briandfoy briandfoy Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can also get rid of the quotes around the filename

bin/tee Outdated

for (@ARGV) {
if (!open($fh, (/^[^>|]/ && $mode) . $_)) {
if (!open($fh, '<', (/^[^>|]/ && $mode) . $_)) {
Copy link
Owner

@briandfoy briandfoy Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check this

bin/uuencode Outdated

local *INPUT;
open(INPUT, "< $source") || die "can't open $source: $!";
open(INPUT, '<', "< $source") || die "can't open $source: $!";
Copy link
Owner

@briandfoy briandfoy Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check this

bin/wc Outdated
if (@ARGV) {
foreach my $filename (@ARGV) {
open(FH, "< $filename") or die "cannot open $filename: $!\n";
open(FH, '<', "< $filename") or die "cannot open $filename: $!\n";
Copy link
Owner

@briandfoy briandfoy Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check this

@IvanLudvig
Copy link
Author

IvanLudvig commented Oct 21, 2019

I'm sorry for mixing it all up. Thanks for pointing out every mistake. I've added corrections

@coveralls
Copy link

coveralls commented Oct 21, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 102

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.006%) to 91.638%
Totals Coverage Status
Change from base Build 94: 0.006%
Covered Lines: 526
Relevant Lines: 574
💛 - Coveralls

briandfoy
briandfoy requested changes Oct 21, 2019
View reviewed changes
Copy link
Owner

@briandfoy briandfoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more changes. These sort of changes are tricky. :)

bin/tee Outdated

for (@ARGV) {
if (!open($fh, (/^[^>|]/ && $mode) . $_)) {
if (!open($fh, '/^[^>|]/', ($mode) . $_)) {
Copy link
Owner

@briandfoy briandfoy Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is suspect. Maybe leave this one alone.

Not that the stuff in parens is trying to determine the mode then append that to $_. The mode is not the pattern /^[^>|]/.

bin/what Outdated
foreach $file (@ARGV)
{
open(FILE, "<$file") or die "Unable to read $file: $!";
open(FILE, '<', "<$file") or die "Unable to read $file: $!";
Copy link
Owner

@briandfoy briandfoy Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is wrong.

bin/awk Outdated

open(TMPOUT,"+> " . ($tmpout = "a2pout.$$")) ||
open(TMPOUT,"+> " . ($tmpout = "/tmp/a2pout.$$")) ||
open(TMPOUT, "+> ", ($tmpout = "a2pout.$$")) ||
Copy link
Owner

@briandfoy briandfoy Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

take the spaces out of the mode.

bin/awk Outdated
open(TMPOUT,"+> " . ($tmpout = "a2pout.$$")) ||
open(TMPOUT,"+> " . ($tmpout = "/tmp/a2pout.$$")) ||
open(TMPOUT, "+> ", ($tmpout = "a2pout.$$")) ||
open(TMPOUT, "+> " , ($tmpout = "/tmp/a2pout.$$")) ||
Copy link
Owner

@briandfoy briandfoy Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

take the spaces out of the mode.

@IvanLudvig
Copy link
Author

IvanLudvig commented Oct 21, 2019

Did the changes

@briandfoy
Copy link
Owner

briandfoy commented Nov 5, 2019

I'm tracking this, but I want to improve the tests to make sure everything happened correctly. That's just a limit on my time. I think the changes are good, but I also know that bugs don't care what I think. :)

@briandfoy
Copy link
Owner

briandfoy commented Feb 16, 2020

I manually applied many of these changes because I needed to adjust other things to make them work out. Thanks for the contributions!

@briandfoy briandfoy closed this Feb 16, 2020
@briandfoy briandfoy self-assigned this Jul 11, 2023
@briandfoy briandfoy added Status: released there is a new release with this fix Type: modernization updating programs to current practices Program: grep The grep program Program: split The split program Program: sum The sum program Program: awk The awk program Program: fold The fold program Program: hangman The hangman program Program: look The look program Program: shar The shar program Program: uuencode The uuencode program Program: par The par program labels Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@briandfoy briandfoy briandfoy requested changes

Assignees

@briandfoy briandfoy

Labels

Program: awk The awk program Program: fold The fold program Program: grep The grep program Program: hangman The hangman program Program: look The look program Program: par The par program Program: shar The shar program Program: split The split program Program: sum The sum program Program: uuencode The uuencode program Status: released there is a new release with this fix Type: modernization updating programs to current practices

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@IvanLudvig @coveralls @briandfoy