a bunch of changes, all related to error-handling:

- have Handle->register also create a proper END block, so that
individual packages don't have to, and explain the issue
- kill old Unlink/Copy code that migrated to State years ago
- commonalize try{} catch {} for pkg_add/delete and pkg_create, so that
debug works the same way in both.
- switch printing command name to the catch handler, so that exceptions
are simpler to handle

and a few comments for the hairy parts...

7 files changed, 124 insertions, 188 deletions

diff --git a/usr.sbin/pkg_add/OpenBSD/AddCreateDelete.pm b/usr.sbin/pkg_add/OpenBSD/AddCreateDelete.pm
index cfb937e9c5c..17157bd53a8 100644
--- a/usr.sbin/pkg_add/OpenBSD/AddCreateDelete.pm
+++ b/usr.sbin/pkg_add/OpenBSD/AddCreateDelete.pm
@@ -1,5 +1,5 @@
 # ex:ts=8 sw=4:
-# $OpenBSD: AddCreateDelete.pm,v 1.44 2019/06/09 18:58:06 espie Exp $
+# $OpenBSD: AddCreateDelete.pm,v 1.45 2019/07/14 07:27:18 espie Exp $
 #
 # Copyright (c) 2007-2014 Marc Espie <espie@openbsd.org>
 #
@@ -187,6 +187,25 @@ sub handle_options
 	$state->handle_options($opt_string, $self, @usage);
 }
 
+sub try_and_run_command
+{
+	my ($self, $state) = @_;
+	if ($state->defines('debug')) {
+		$self->run_command($state);
+	} else {
+		try {
+			$self->run_command($state);
+		} catch {
+			$state->errsay("#1: #2", $state->{cmd}, $_);
+			OpenBSD::Handler->reset;
+			if ($_ =~ m/^Caught SIG(\w+)/o) {
+				kill $1, $$;
+			}
+			$state->{bad}++;
+		};
+	}
+}
+
 package OpenBSD::InteractiveStub;
 sub new
 {
diff --git a/usr.sbin/pkg_add/OpenBSD/AddDelete.pm b/usr.sbin/pkg_add/OpenBSD/AddDelete.pm
index e1a43e04809..e7609def601 100644
--- a/usr.sbin/pkg_add/OpenBSD/AddDelete.pm
+++ b/usr.sbin/pkg_add/OpenBSD/AddDelete.pm
@@ -1,5 +1,5 @@
 # ex:ts=8 sw=4:
-# $OpenBSD: AddDelete.pm,v 1.87 2019/07/11 07:14:23 espie Exp $
+# $OpenBSD: AddDelete.pm,v 1.88 2019/07/14 07:27:18 espie Exp $
 #
 # Copyright (c) 2007-2010 Marc Espie <espie@openbsd.org>
 #
@@ -63,6 +63,7 @@ use OpenBSD::Error;
 use OpenBSD::Paths;
 use OpenBSD::PackageInfo;
 use OpenBSD::AddCreateDelete;
+our @ISA = qw(OpenBSD::AddCreateDelete);
 
 sub do_the_main_work
 {
@@ -102,49 +103,32 @@ sub handle_end_tags
 	    });
 }
 
-sub framework
+sub run_command
 {
 	my ($self, $state) = @_;
 
-	my $do = sub {
-		lock_db($state->{not}, $state) unless $state->defines('nolock');
-		$state->check_root;
-		$self->process_parameters($state);
-		my $dielater = $self->do_the_main_work($state);
-		# cleanup various things
-		$self->handle_end_tags($state);
-		$state->{recorder}->cleanup($state);
-		$state->ldconfig->ensure;
-		OpenBSD::PackingElement->finish($state);
-		$state->progress->clear;
-		$state->log->dump;
-		$self->finish_display($state);
-		if ($state->verbose >= 2 || $state->{size_only} ||
-		    $state->defines('tally')) {
-			$state->vstat->tally;
-		}
-		$state->say("Extracted #1 from #2", 
-		    $state->{stats}{donesize},
-		    $state->{stats}{totsize}) 
-			if defined $state->{stats} and $state->verbose;
-		# show any error, and show why we died...
-		rethrow $dielater;
-	};
-	if ($state->defines('debug')) {
-		&$do;
-	} else {
-		try {
-			&$do;
-		} catch {
-			$state->errsay("#1", $_);
-			OpenBSD::Handler->reset;
-			if ($_ =~ m/^Caught SIG(\w+)/o) {
-				kill $1, $$;
-			}
-			$state->{bad}++;
-		};
+	lock_db($state->{not}, $state) unless $state->defines('nolock');
+	$state->check_root;
+	$self->process_parameters($state);
+	my $dielater = $self->do_the_main_work($state);
+	# cleanup various things
+	$self->handle_end_tags($state);
+	$state->{recorder}->cleanup($state);
+	$state->ldconfig->ensure;
+	OpenBSD::PackingElement->finish($state);
+	$state->progress->clear;
+	$state->log->dump;
+	$self->finish_display($state);
+	if ($state->verbose >= 2 || $state->{size_only} ||
+	    $state->defines('tally')) {
+		$state->vstat->tally;
 	}
-
+	$state->say("Extracted #1 from #2", 
+	    $state->{stats}{donesize},
+	    $state->{stats}{totsize}) 
+		if defined $state->{stats} and $state->verbose;
+	# show any error, and show why we died...
+	rethrow $dielater;
 }
 
 sub parse_and_run
@@ -174,7 +158,7 @@ sub parse_and_run
 		}
 	}
 
-	$self->framework($state);
+	$self->try_and_run_command($state);
 
 	if (defined $lflag) {
 		$termios->setlflag($lflag);
diff --git a/usr.sbin/pkg_add/OpenBSD/Error.pm b/usr.sbin/pkg_add/OpenBSD/Error.pm
index 1148a84c7f5..90451d1573c 100644
--- a/usr.sbin/pkg_add/OpenBSD/Error.pm
+++ b/usr.sbin/pkg_add/OpenBSD/Error.pm
@@ -1,5 +1,5 @@
 # ex:ts=8 sw=4:
-# $OpenBSD: Error.pm,v 1.33 2019/07/11 07:03:45 espie Exp $
+# $OpenBSD: Error.pm,v 1.34 2019/07/14 07:27:18 espie Exp $
 #
 # Copyright (c) 2004-2010 Marc Espie <espie@openbsd.org>
 #
@@ -17,6 +17,8 @@
 use strict;
 use warnings;
 
+# this is a set of common classes related to error handling in pkg land
+
 package OpenBSD::Auto;
 sub cache(*&)
 {
@@ -30,32 +32,55 @@ sub cache(*&)
 	*{$callpkg."::$sym"} = $actual;
 }
 
+# a bunch of other modules create persistent state that must be cleaned up
+# on exit (temporary files, network connections to abort properly...)
+# END blocks would do that (but see below...) but sig handling bypasses that,
+# so we MUST install SIG handlers.
+
+# note that END will be run for *each* process, so beware!
+# (temp files are registered per pid, for instance, so they only
+# get cleaned when the proper pid is used)
 package OpenBSD::Handler;
 
-my $list = [];
+# hash of code to run on ANY exit
+my $cleanup = {};
+
+sub cleanup
+{
+	my ($class, $sig) = @_;
+	# XXX note that order of cleanup is "unpredictable"
+	for my $v (values %$cleanup) {
+		&$v($sig);
+	}
+}
 
+END {
+	# XXX localize $? so that cleanup doesn't fuck up our exit code
+	local $?;
+	cleanup();
+}
+
+# register each code block "by name" so that we can re-register each
+# block several times
 sub register
 {
 	my ($class, $code) = @_;
-	push(@$list, $code);
+	$cleanup->{$code} = $code;
 }
 
 my $handler = sub {
 	my $sig = shift;
-	for my $c (@$list) {
-		&$c($sig);
-	}
+	__PACKAGE__->cleanup($sig);
+	# after cleanup, just propagate the signal
 	$SIG{$sig} = 'DEFAULT';
 	kill $sig, $$;
 };
 
 sub reset
 {
-	$SIG{'INT'} = $handler;
-	$SIG{'QUIT'} = $handler;
-	$SIG{'HUP'} = $handler;
-	$SIG{'KILL'} = $handler;
-	$SIG{'TERM'} = $handler;
+	for my $sig (qw(INT QUIT HUP KILL TERM)) {
+		$SIG{$sig} = $handler;
+	}
 }
 
 __PACKAGE__->reset;
@@ -63,95 +88,11 @@ __PACKAGE__->reset;
 package OpenBSD::Error;
 require Exporter;
 our @ISA=qw(Exporter);
-our @EXPORT=qw(Copy Unlink try throw catch catchall rethrow);
+our @EXPORT=qw(try throw catch catchall rethrow);
 
 our ($FileName, $Line, $FullMessage);
 
-my @signal_name = ();
-
 use Carp;
-
-sub fillup_names
-{
-	{
-	# XXX force autoload
-	package verylocal;
-
-	require POSIX;
-	POSIX->import(qw(signal_h));
-	}
-
-	for my $sym (keys %POSIX::) {
-		next unless $sym =~ /^SIG([A-Z].*)/;
-		my $i = eval "&POSIX::$sym()";
-		next unless defined $i;
-		$signal_name[$i] = $1;
-	}
-	# extra BSD signals
-	$signal_name[5] = 'TRAP';
-	$signal_name[7] = 'IOT';
-	$signal_name[10] = 'BUS';
-	$signal_name[12] = 'SYS';
-	$signal_name[16] = 'URG';
-	$signal_name[23] = 'IO';
-	$signal_name[24] = 'XCPU';
-	$signal_name[25] = 'XFSZ';
-	$signal_name[26] = 'VTALRM';
-	$signal_name[27] = 'PROF';
-	$signal_name[28] = 'WINCH';
-	$signal_name[29] = 'INFO';
-}
-
-sub find_signal
-{
-	my $number =  shift;
-
-	if (@signal_name == 0) {
-		fillup_names();
-	}
-
-	return $signal_name[$number] || $number;
-}
-
-sub child_error
-{
-	my $error = shift // $?;
-
-	my $extra = "";
-
-	if ($error & 128) {
-		$extra = " (core dumped)";
-	}
-	if ($error & 127) {
-		return "killed by signal ". find_signal($error & 127).$extra;
-	} else {
-		return "exit(". ($error >> 8) . ")$extra";
-	}
-}
-
-sub Copy
-{
-	require File::Copy;
-
-	my $r = File::Copy::copy(@_);
-	if (!$r) {
-		print "copy(", join(',', @_),") failed: $!\n";
-	}
-	return $r;
-}
-
-sub Unlink
-{
-	my $verbose = shift;
-	my $r = unlink @_;
-	if ($r != @_) {
-		print "rm @_ failed: removed only $r targets, $!\n";
-	} elsif ($verbose) {
-		print "rm @_\n";
-	}
-	return $r;
-}
-
 sub dienow
 {
 	my ($error, $handler) = @_;
diff --git a/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm b/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm
index e19d2eceaa9..25bd6b9a401 100644
--- a/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm
+++ b/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm
@@ -1,5 +1,5 @@
 # ex:ts=8 sw=4:
-# $OpenBSD: PackageRepository.pm,v 1.164 2019/07/10 09:34:38 espie Exp $
+# $OpenBSD: PackageRepository.pm,v 1.165 2019/07/14 07:27:18 espie Exp $
 #
 # Copyright (c) 2003-2010 Marc Espie <espie@openbsd.org>
 #
@@ -82,18 +82,12 @@ sub unique
 	return $o;
 }
 
-my $cleanup = sub {
+OpenBSD::Handler->register(
+    sub {
 	for my $repo (values %$cache) {
 		$repo->cleanup;
 	}
-};
-END {
-	my $a = $?;
-	&$cleanup;
-	$? = $a;
-}
-
-OpenBSD::Handler->register($cleanup);
+    });
 
 sub parse_fullurl
 {
diff --git a/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm b/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm
index 4f94b9e4b31..ecebae95f13 100644
--- a/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm
+++ b/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm
@@ -1,6 +1,6 @@
 #! /usr/bin/perl
 # ex:ts=8 sw=4:
-# $OpenBSD: PkgCreate.pm,v 1.160 2019/07/11 07:14:23 espie Exp $
+# $OpenBSD: PkgCreate.pm,v 1.161 2019/07/14 07:27:18 espie Exp $
 #
 # Copyright (c) 2003-2014 Marc Espie <espie@openbsd.org>
 #
@@ -1587,27 +1587,9 @@ sub save_history
 	return $l;
 }
 
-sub parse_and_run
+sub run_command
 {
-	my ($self, $cmd) = @_;
-
-	my $regen_package = 0;
-	my $sign_only = 0;
-	my $rc = 0;
-
-	my $state = OpenBSD::PkgCreate::State->new($cmd);
-	$state->handle_options;
-
-	if (@ARGV == 0) {
-		$regen_package = 1;
-	} elsif (@ARGV != 1) {
-		if (defined $state->{contents} || 
-		    !defined $state->{signature_params}) {
-			$state->usage("Exactly one single package name is required: #1", join(' ', @ARGV));
-		}
-	}
-
-	try {
+	my ($self, $state) = @_;
 	if (defined $state->opt('Q')) {
 		$state->{opt}{q} = 1;
 	}
@@ -1617,7 +1599,7 @@ sub parse_and_run
 	}
 
 	my $plist;
-	if ($regen_package) {
+	if ($state->{regen_package}) {
 		if (!defined $state->{contents} || @{$state->{contents}} > 1) {
 			$state->usage("Exactly one single packing-list is required");
 		}
@@ -1641,7 +1623,7 @@ sub parse_and_run
 			$plist->stub_digest($ordered);
 		} else {
 			$state->set_status("checksumming");
-			if ($regen_package) {
+			if ($state->{regen_package}) {
 				$state->progress->visit_with_count($plist, 
 				    'verify_checksum');
 			} else {
@@ -1684,7 +1666,7 @@ sub parse_and_run
 	$state->{bad} = 0;
 
 	my $wname;
-	if ($regen_package) {
+	if ($state->{regen_package}) {
 		$wname = $plist->pkgname.".tgz";
 	} else {
 		$plist->save or $state->fatal("can't write packing-list");
@@ -1701,11 +1683,29 @@ sub parse_and_run
 	if (!$state->defines("stub")) {
 		$self->finish_manpages($state, $plist);
 	}
-	} catch {
-		$state->errsay("#1", $_);
-		$rc = 1;
-	};
-	return $rc;
+}
+
+sub parse_and_run
+{
+	my ($self, $cmd) = @_;
+
+	my $sign_only = 0;
+	my $rc = 0;
+
+	my $state = OpenBSD::PkgCreate::State->new($cmd);
+	$state->handle_options;
+
+	if (@ARGV == 0) {
+		$state->{regen_package} = 1;
+	} elsif (@ARGV != 1) {
+		if (defined $state->{contents} || 
+		    !defined $state->{signature_params}) {
+			$state->usage("Exactly one single package name is required: #1", join(' ', @ARGV));
+		}
+	}
+
+	$self->try_and_run_command($state);
+	return $state->{bad} != 0;
 }
 
 1;
diff --git a/usr.sbin/pkg_add/OpenBSD/State.pm b/usr.sbin/pkg_add/OpenBSD/State.pm
index 6f4172add23..1c3a752723d 100644
--- a/usr.sbin/pkg_add/OpenBSD/State.pm
+++ b/usr.sbin/pkg_add/OpenBSD/State.pm
@@ -1,5 +1,5 @@
 # ex:ts=8 sw=4:
-# $OpenBSD: State.pm,v 1.58 2019/07/11 07:14:23 espie Exp $
+# $OpenBSD: State.pm,v 1.59 2019/07/14 07:27:18 espie Exp $
 #
 # Copyright (c) 2007-2014 Marc Espie <espie@openbsd.org>
 #
@@ -214,7 +214,7 @@ sub _fatal
 	# the way is to eval { croak @_}; and decide what to do with $@.
 	delete $SIG{__DIE__};
 	$self->sync_display;
-	croak $self->{cmd}, ": ", @_, "\n";
+	croak @_, "\n";
 }
 
 sub fatal
diff --git a/usr.sbin/pkg_add/OpenBSD/Temp.pm b/usr.sbin/pkg_add/OpenBSD/Temp.pm
index 7da540ebbd8..db695c1e158 100644
--- a/usr.sbin/pkg_add/OpenBSD/Temp.pm
+++ b/usr.sbin/pkg_add/OpenBSD/Temp.pm
@@ -1,5 +1,5 @@
 # ex:ts=8 sw=4:
-# $OpenBSD: Temp.pm,v 1.34 2019/07/10 09:34:09 espie Exp $
+# $OpenBSD: Temp.pm,v 1.35 2019/07/14 07:27:18 espie Exp $
 #
 # Copyright (c) 2003-2005 Marc Espie <espie@openbsd.org>
 #
@@ -26,26 +26,24 @@ use OpenBSD::Error;
 
 our $tempbase = $ENV{'PKG_TMPDIR'} || OpenBSD::Paths->vartmp;
 
+# stuff that should be cleaned up on exit, registered by pid,
+# so that it gets cleaned on exit from the correct process
+
 my $dirs = {};
 my $files = {};
 
 my ($lastname, $lasterror, $lasttype);
 
-my $cleanup = sub {
+OpenBSD::Handler->register(
+    sub {
 	while (my ($name, $pid) = each %$files) {
 		unlink($name) if $pid == $$;
 	}
 	while (my ($dir, $pid) = each %$dirs) {
 		OpenBSD::Error->rmtree([$dir]) if $pid == $$;
 	}
-};
+    });
 
-END {
-	my $r = $?;
-	&$cleanup;
-	$? = $r;
-}
-OpenBSD::Handler->register($cleanup);
 
 sub dir
 {