Pass critic's stern settings

Convert return undef; to return;

Convert 2 arg open to 3 arg open

Add explicit return at end of functions

Don't mix high and low-precedence booleans

Convert setting signal handlers to use POSIX::sigaction

Convert grep to use block form

Style, whitespace after comma

Use die with strings ending with newline

Style, whitespace

autoupdate.pl

@@ -15,6 +15,8 @@
 # ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 # OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
+## no critic (RequireBriefOpen)
+
 use strict;
 use warnings;
 use Cwd;
@@ -24,12 +26,7 @@ use OpenBSD::PackageName;
 use Getopt::Long;
 use FindBin;
 use File::Spec;
-use POSIX qw/uname :sys_wait_h/;
-
-# Silence the warning that is issued because List::Util won't register
-# $a and $b in a correct way for us
-$a = $a;
-$b = $b;
+use POSIX qw/uname :sys_wait_h :signal_h/;
 
 # Defaults to 1, can be set via the --verbose switch
 my $verbose = 1;
@@ -90,6 +87,11 @@ my @aborted = ();
 # Hash of reaped child processes
 my %reaped_pids = ();
 
+# pipe to tee for logging
+my $tee = undef;
+
+# Default signal handler for SIGCHLD
+my $old_child_sigaction = POSIX::SigAction->new();
 
 # This function will remove the finished forked build from the list of
 # currently build ports.
@@ -100,7 +102,7 @@ sub REAPER {
 		$abort = 1 if ($status != 0);
 		$reaped_pids{$wpid} = $status;
 	}
-	$SIG{CHLD} = \&REAPER;
+	return;
 }
 
 
@@ -129,6 +131,7 @@ sub reap {
 		}
 		delete $reaped_pids{$wpid};
 	}
+	return;
 }
 
 
@@ -179,7 +182,7 @@ sub read_rc_file {
 		if (! m/=/) {
 			print STDERR 'Not a valid config in line '
 			    . $real_lineno . "\n";
-			return undef;
+			return;
 		}
 
 		# Split the line at the first '='
@@ -214,6 +217,7 @@ sub setup_lookup_hash {
 			$hash->{$_} = 1;
 		}
 	}
+	return;
 }
 
 
@@ -226,16 +230,17 @@ sub setup_logging {
 		$logdir = tempdir("$tmpdir/autoupdate.XXXXXXXXXX");
 
 		my $logfile = "$logdir/autoupdate.pl.log";
-		open TEE, "| tee $logfile"
-		    or die "Unable to open log file \"$logfile\"\n";
-		open STDOUT, ">&TEE"
+		open $tee, '|-', "tee $logfile"
+		    or die qq{Unable to open log file "$logfile": $!\n};
+		open STDOUT, '>&', $tee
 		    or die "Unable to redirect STDOUT to log file.\n";
-		open STDERR, ">&TEE"
+		open STDERR, '>&', $tee
 		    or die "Unable to redirect STDERR to log file.\n";
 
 		print STDOUT 'Logging builds in "' . $logdir . '"' . "\n";
 		print STDOUT 'Using "' . $logfile . '" as mainlog' . "\n";
 	}
+	return;
 }
 
 
@@ -243,7 +248,7 @@ sub setup_logging {
 sub get_ports_version {
 	my $port = shift;
 
-	chdir "$port" || return undef;
+	chdir "$port" || return;
 	my $show = 'show';
 	my @uname = POSIX::uname();
 	if ($uname[2] > 6.4) {
@@ -252,7 +257,7 @@ sub get_ports_version {
 	my $cmd = "make $show=FULLPKGNAME";
 	my $pkgname = '';
 
-	open(my $in, "$cmd 2>&1 |")
+	open(my $in, '-|', "$cmd 2>&1")
 	    or die "Unable to get version for \"$port\"\n";
 	while (<$in>) {
 		chomp;
@@ -281,8 +286,8 @@ sub get_higher_version {
 		return -1;
 	}
 
-	my @vers1 = split /[\.pv]/,$ver1;
-	my @vers2 = split /[\.pv]/,$ver2;
+	my @vers1 = split /[\.pv]/, $ver1;
+	my @vers2 = split /[\.pv]/, $ver2;
 
 	my $max_len = scalar @vers1;
 	$max_len = scalar @vers2 if (scalar @vers2 > scalar @vers1);
@@ -312,7 +317,7 @@ sub read_update_package_list {
 	my ($in, @package_list, $out);
 
 	# Save the output from pkg_outdated in a log file
-	if (defined $logdir && not defined $input) {
+	if (defined $logdir && !defined $input) {
 		open($out, '>', "$logdir/pkg_outdated");
 		print STDERR "Warning: Unable to open log for pkg_outdated\n"
 		    if (not defined $out);
@@ -322,13 +327,13 @@ sub read_update_package_list {
 		my $ood_path = 'infrastructure/bin';
 		my $cmd = "env PORTSDIR=\"$portsdir\" "
 		    . "\"$portsdir/$ood_path/pkg_outdated\" ";
-		unless (open($in, $cmd . '2>/dev/null |')) {
+		unless (open($in, '-|', "$cmd 2>/dev/null")) {
 			print STDERR "Unable to execute $cmd\n";
 			exit 1;
 		}
 	}
 	elsif ($input eq '-') {
-		unless (open($in, '<&=STDIN')) {
+		unless (open($in, '<&=', \*STDIN)) {
 			print STDERR "Unable to open stdin\n";
 			exit 1;
 		}
@@ -416,9 +421,9 @@ sub create_package_information {
 				category => $category,
 				port     => $port,
 				subdir   => $subdir,
-				subpkg	 => $subpackage,
-				flavor	 => $flavor,
-				pkg	 => $pkg,
+				subpkg   => $subpackage,
+				flavor   => $flavor,
+				pkg      => $pkg,
 			);
 			$p{dependencies} = create_dependencies_list(\%p);
 			$p{jobs} = set_parallel_build(\%p);
@@ -459,6 +464,7 @@ sub add_pseudo_flavors {
 		}
 	}
 	$info->{flavor} = $flavors;
+	return;
 }
 
 
@@ -472,10 +478,10 @@ sub create_pseudo_flavors_list {
 
 	# chdir into the ports directory
 	my ($port, $port_dir) = find_newer_ports_dir($info);
-	chdir $port_dir or die "Unable to change to \"$port_dir\"";
+	chdir $port_dir or die qq{Unable to change to "$port_dir": $!\n};
 
-	open (my $in, "$cmd |")
-	    or die "Unable to get pseudo flavors for \"$port\"";
+	open (my $in, '-|', "$cmd")
+	    or die qq{Unable to get pseudo flavors for "$port"\n};
 
 	my $output;
 	while (<$in>) {
@@ -485,10 +491,10 @@ sub create_pseudo_flavors_list {
 	chop $output;
 	close($in);
 	if ($? != 0) {
-		die "Unable to get pseudo flavors for \"$port\"";
+		die qq{Unable to get pseudo flavors for "$port\n"};
 	}
 
-	my @pseudo_flavors = split / /,$output;
+	my @pseudo_flavors = split / /, $output;
 	return \@pseudo_flavors;
 }
 
@@ -523,7 +529,7 @@ sub find_newer_ports_dir {
 	return ($port, $info->{portdir})
 	    if ((defined $info->{portdir}) && ($info->{portdir} ne ''));
 
-	my @port_locations = split /:/,$portsdir_path;
+	my @port_locations = split /:/, $portsdir_path;
 	# Take a shortcut here if we got only one location
 	if (scalar @port_locations == 1) {
 		$info->{portdir} = "$portsdir_path/$port";
@@ -568,8 +574,8 @@ sub build_pkg {
 		return;
 	}
 
-	# The child must use the default sig handler for SIGCHLD
-	$SIG{CHLD} = 'DEFAULT';
+	POSIX::sigaction(&POSIX::SIGCHLD, $old_child_sigaction)
+	    or die "Unable to set SIGCHLD handler: $!\n";
 
 	# Give the parent time to update the forked_builds hash
 	sleep 1;
@@ -586,7 +592,7 @@ sub build_pkg {
 
 	print STDOUT "Building $info->{pkg}\n";
 	my (@log, $logfile, $logfilename);
-	open(my $in, "$cmd 2>&1 |")
+	open(my $in, '-|', "$cmd 2>&1")
 	    or die "Unable to make \"$port\"\n";
 	if (defined $logdir) {
 		my $tmp = $info->{port};
@@ -616,7 +622,7 @@ sub build_pkg {
 		$cmd .= "SUBPACKAGE=$info->{subpkg} "
 		    if ($info->{subpkg} ne '');
 		$cmd .= ' make repackage update clean';
-		open($in, "$cmd 2>&1 |")
+		open($in, '-|', "$cmd 2>&1")
 		    or die "Unable to update \"$port\"\n";
 		while (<$in>) {
 			print STDOUT $_
@@ -652,13 +658,12 @@ sub set_parallel_build {
 	my $info = shift;
 
 	my $cur_dir = getcwd;
-	my $cmd = 'make show=DPB_PROPERTIES';
 	my ($port, $port_dir) = find_newer_ports_dir($info);
-	chdir $port_dir or die "Unable to change to '$port_dir'\n";
+	chdir $port_dir or die qq{Unable to change to "$port_dir"\n};
 
-	open(my $in, "$cmd |")
-	    or die "Unable to determine parallel build info for \"$port\"\n";
-	my $parallel = grep(/parallel/, (<$in>));
+	open(my $in, '-|', 'make', 'show=DPB_PROPERTIES')
+	    or die qq{Unable to determine parallel build info for "$port"\n};
+	my $parallel = grep { /parallel/x } (<$in>);
 	close($in);
 	chdir $cur_dir;
 
@@ -685,7 +690,7 @@ sub create_dependencies_list {
 	my ($port, $port_dir) = find_newer_ports_dir($info);
 	chdir $port_dir or die "Unable to change to \"$port_dir\"\n";
 
-	open(my $in, "$cmd |")
+	open(my $in, '-|', "$cmd")
 	    or die "Unable to get dependencies for \"$port\"\n";
 	while (<$in>) {
 		chomp;
@@ -693,7 +698,7 @@ sub create_dependencies_list {
 	}
 	close($in);
 	if ($? != 0) {
-		die "Unable to gather information for $port";
+		die "Unable to gather information for $port\n";
 	}
 
 	# Add the port itself to the list
@@ -778,8 +783,6 @@ sub usage {
 
 ##############################################################################
 
-$SIG{CHLD} = 'DEFAULT';
-
 # Read the command line params
 my $result = GetOptions("v|verbose=i" => \$verbose,
 			"f|outdated=s" => \$out_of_date,
@@ -828,7 +831,7 @@ foreach my $info (@$port_info) {
 print STDOUT "Create dependencies...\n";
 my ($fh, $file, $pipe);
 ($fh, $file) = tempfile(UNLINK => 1);
-open($pipe, "| tsort -r >$file") or die "ERROR: Unable to spawn tsort\n";
+open($pipe, '|-', "tsort -r >$file") or die "ERROR: Unable to spawn tsort\n";
 foreach my $dep_entry (@dep_list) {
 	# Zap the flavors and the subpackages for our dep list
 	$dep_entry =~ s/$regex_subpkg$regex_flavor//g;
@@ -836,7 +839,7 @@ foreach my $dep_entry (@dep_list) {
 	print $pipe "$dep_entry\n";
 }
 close $pipe;
-die "ERROR: Internal error" if ($? != 0);
+die "ERROR: Internal error\n" if ($? != 0);
 
 # Read the result
 while (<$fh>) {
@@ -865,7 +868,13 @@ if ($verbose > 1) {
 	}
 }
 
-$SIG{CHLD} = \&REAPER;
+POSIX::sigaction(
+	&POSIX::SIGCHLD,
+	POSIX::SigAction->new(
+		\&REAPER, POSIX::SigSet->new(), &POSIX::SA_NODEFER
+	),
+	$old_child_sigaction
+) or die "Unable to set SIGCHLD signal handler: $!\n";
 
 # Build all the packages. Packages that are still unknown to us are ignored.
 foreach my $pkg_name (@pkg_list)  {
@@ -906,16 +915,12 @@ while (scalar (keys %forked_builds)) {
 	reap();
 }
 
-$SIG{CHLD} = 'DEFAULT';
-
-# Close the pipe to our log file if we were logging
-if (defined $config->{logging} && $config->{logging} != 0) {
-	close STDOUT;
-	close STDERR;
-	close TEE;
-}
+POSIX::sigaction(
+	&POSIX::SIGCHLD,
+	$old_child_sigaction
+) or warn "Unable to set SIGCHLD signal handler: $!\n";
 
-die 'Abort requested by child' if ($abort != 0);
+die "Abort requested by child\n" if ($abort != 0);
 print STDOUT "Done.\n";
 
 exit 0;