>Number:         6010
>Category:       pkg
>Synopsis:       most fatal errors in pkg_install progs are unreachable
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    gnats-admin (GNATS administrator)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Aug 23 20:50:00 1998
>Last-Modified:
>Originator:     Jim Bernard
>Organization:
	Speaking for myself
>Release:        August 22, 1998
>Environment:
System: NetBSD zoo 1.3F NetBSD 1.3F (ZOO) #0: Sun Jun 14 09:09:08 MDT 1998 local@zoo:/home/local/compile/sys/arch/i386/compile/ZOO i386


>Description:
	There are numerous places in the pkg_install programs where
	fatal errors are trapped, the cleanup() function is called,
	and the code intends then to exit with an error message generated
	by errx.  However, all the cleanup() functions have had an
	exit(1) stuck in them, so they never return and the error messages
	are never printed.  Not very friendly.

>How-To-Repeat:
	Do something that generates an error and wonder what went wrong
	and why no information at all was provided on the nature of the
	problem.  E.g., if the patches from PR 6009 have not been applied
	(adds support for symbolic links in PREFIX), make PREFIX a symbolic
	link and try to pkg_add a binary package.  It will completely silently
	fail to add the package, giving no clue as to the problem.

>Fix:
	These patches remove the exit() call from the various cleanup()
	functions, except in the case where they are called in response
	to receipt of a signal.  All the changed files are in subdirs of
	usr.sbin/pkg_install (lib/msg.c, add/perform.c, create/perform.c,
	delete/perform.c, and info/perform.c).

	The lib/msg.c file contained some calls to cleanup that _did_ assume
	that cleanup() would exit, in contrast to the multitude of other
	calls to cleanup(), so these were changed to expect it not to exit.

	There was originally protection against multiple calls to cleanup()
	in add/perform.c, on account of the fact that leave_playpen(),
	which is called by cleanup(), can, in turn, call cleanup().  I
	extended that to the other cleanup() functions, where they too
	called leave_playpen.

	In addition, there's the (small) possibility of a signal being
	received while cleanup() is in progress, so in those programs
	where signals were being caught, I changed cleanup() to ignore
	them during its execution (in every case the code exits shortly
	after cleanup(), so there should be no problem with this).

	After applying these patches, voila! you will see error messages
	again.

--- lib/msg.c-dist	Fri Oct 17 13:24:52 1997
+++ lib/msg.c	Sun Aug 23 20:58:05 1998
@@ -33,14 +33,14 @@
 #include <err.h>
 #include "lib.h"
 
 /* Die a relatively simple death */
 void
-upchuck(const char *err)
+upchuck(const char *errstr)
 {
-    warn("fatal error during execution: %s", err);
     cleanup(0);
+    err(1, "fatal error during execution: %s", errstr);
 }
 
 /*
  * As a yes/no question, prompting from the varargs string and using
  * default if user just hits return.
@@ -57,12 +57,12 @@
      * Need to open /dev/tty because file collection may have been
      * collected on stdin
      */
     tty = fopen("/dev/tty", "r");
     if (!tty) {
-	warnx("can't open /dev/tty!");
 	cleanup(0);
+	errx(1, "can't open /dev/tty!");
     }
     while (ch != 'Y' && ch != 'N') {
 	vfprintf(stderr, msg, args);
 	if (def)
 	    fprintf(stderr, " [yes]? ");
--- add/perform.c-dist	Mon Jul 13 05:17:48 1998
+++ add/perform.c	Sun Aug 23 08:37:26 1998
@@ -493,16 +493,23 @@
 
 void
 cleanup(int signo)
 {
     static int	alreadyCleaning;
+    void (*oldint)(int);
+    void (*oldhup)(int);
+    oldint = signal(SIGINT, SIG_IGN);
+    oldhup = signal(SIGHUP, SIG_IGN);
 
     if (!alreadyCleaning) {
     	alreadyCleaning = 1;
 	if (signo)
-		printf("Signal %d received, cleaning up..\n", signo);
+	    printf("Signal %d received, cleaning up..\n", signo);
 	if (!Fake && LogDir[0])
-		vsystem("%s -rf %s", REMOVE_CMD, LogDir);
+	    vsystem("%s -rf %s", REMOVE_CMD, LogDir);
 	leave_playpen(Home);
+	if (signo)
+	    exit(1);
     }
-    exit(1);
+    signal(SIGINT, oldint);
+    signal(SIGHUP, oldhup);
 }
--- create/perform.c-dist	Sat Jun  6 05:21:57 1998
+++ create/perform.c	Sun Aug 23 20:33:39 1998
@@ -312,8 +312,22 @@
 
 /* Clean up those things that would otherwise hang around */
 void
 cleanup(int sig)
 {
-    leave_playpen(home);
-    exit(1);
+    static int	alreadyCleaning;
+    void (*oldint)(int);
+    void (*oldhup)(int);
+    oldint = signal(SIGINT, SIG_IGN);
+    oldhup = signal(SIGHUP, SIG_IGN);
+
+    if (!alreadyCleaning) {
+    	alreadyCleaning = 1;
+	if (sig)
+	    printf("Signal %d received, cleaning up..\n", sig);
+	leave_playpen(home);
+	if (sig)
+	    exit(1);
+    }
+    signal(SIGINT, oldint);
+    signal(SIGHUP, oldhup);
 }
--- delete/perform.c-dist	Fri Oct 17 13:24:05 1997
+++ delete/perform.c	Sun Aug 23 20:38:44 1998
@@ -166,11 +166,11 @@
 
 void
 cleanup(int sig)
 {
     /* Nothing to do */
-    exit(1);
+    return;
 }
 
 static void
 undepend(PackingList p, char *pkgname)
 {
--- info/perform.c-dist	Fri Jul 10 05:17:07 1998
+++ info/perform.c	Sun Aug 23 20:34:30 1998
@@ -245,8 +245,19 @@
 }
 
 void
 cleanup(int sig)
 {
-    leave_playpen(Home);
-    exit(1);
+    static int	alreadyCleaning;
+    void (*oldint)(int);
+    oldint = signal(SIGINT, SIG_IGN);
+
+    if (!alreadyCleaning) {
+    	alreadyCleaning = 1;
+	if (sig)
+	    printf("Signal %d received, cleaning up..\n", sig);
+	leave_playpen(Home);
+	if (sig)
+	    exit(1);
+    }
+    signal(SIGINT, oldint);
 }
>Audit-Trail:
>Unformatted: