From owner-freebsd-bugs@FreeBSD.ORG Thu Nov 11 06:20:10 2010 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8C7F8106566C for ; Thu, 11 Nov 2010 06:20:10 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 515038FC13 for ; Thu, 11 Nov 2010 06:20:10 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id oAB6KAag045622 for ; Thu, 11 Nov 2010 06:20:10 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id oAB6KAos045621; Thu, 11 Nov 2010 06:20:10 GMT (envelope-from gnats) Resent-Date: Thu, 11 Nov 2010 06:20:10 GMT Resent-Message-Id: <201011110620.oAB6KAos045621@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, "Ronald F.Guilmette" Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 84673106566B for ; Thu, 11 Nov 2010 06:20:02 +0000 (UTC) (envelope-from rfg@tristatelogic.com) Received: from outgoing.tristatelogic.com (segfault.tristatelogic.com [69.62.255.118]) by mx1.freebsd.org (Postfix) with ESMTP id 5B61E8FC19 for ; Thu, 11 Nov 2010 06:20:02 +0000 (UTC) Received: by segfault.tristatelogic.com (Postfix, from userid 1237) id CC954BDC35; Wed, 10 Nov 2010 22:20:01 -0800 (PST) Message-Id: <20101111062001.CC954BDC35@segfault.tristatelogic.com> Date: Wed, 10 Nov 2010 22:20:01 -0800 (PST) From: "Ronald F.Guilmette" To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: Subject: bin/152132: Useless code in script.c (part 2) X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: "Ronald F.Guilmette" List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Nov 2010 06:20:10 -0000 >Number: 152132 >Category: bin >Synopsis: Useless code in script.c (part 2) >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: change-request >Submitter-Id: current-users >Arrival-Date: Thu Nov 11 06:20:09 UTC 2010 >Closed-Date: >Last-Modified: >Originator: Ronald F. Guilmette >Release: FreeBSD 7.0-RELEASE i386 >Organization: Infinite Monkeys & Co. >Environment: System: FreeBSD segfault.tristatelogic.com 7.0-RELEASE FreeBSD 7.0-RELEASE #0: Tue Aug 5 02:38:40 PDT 2008 root@segfault.monkeys.com:/usr/src/sys/i386/compile/rfg20080805 i386 >Description: There appears to be a bunch of utterly useless (and perhaps even time wasting) code in script.c, and specifically within the finish() function. In that function there is a loop that reaps all child processes that are immediately available for reaping. But this must be a leftover from an earlier time when script.c was implemented with -two- child processes. Nowadays, it uses only one child process, i.e. the child shell process. So it makes no sense to loop while trying to reap multiple child processes, because there is at most only one, the child shell process (and so it isn't even necessary or useful to call wait3() because a simple call to wait() should do.) Furthermore, if the idea is to reap the one and only child process, then before the reaping attempt even begins something positive should be done to cause the child to understand that it should exit now, i.e. calling close() on the "master" pty fd, i.e. just prior to all places in the code where the finish() function (or the done() function) is about to be called. That should cause the child (shell) process to receive EOF on stdin/stdout/stderr, and that will probably cause it to figure out that it is time to go bye bye (if it hasn't done so already). >How-To-Repeat: Look at the code. >Fix: *** script.c.orig 2004-02-15 09:30:13.000000000 -0800 --- script.c 2010-11-10 22:02:59.000000000 -0800 *************** *** 155,158 **** --- 155,159 ---- if (child < 0) { warn("fork"); + (void)close(master); done(1); } *************** *** 204,207 **** --- 203,207 ---- } } + (void)close(master); finish(); done(0); *************** *** 220,238 **** { pid_t pid; ! int die, e, status; ! die = e = 0; ! while ((pid = wait3(&status, WNOHANG, 0)) > 0) ! if (pid == child) { ! die = 1; ! if (WIFEXITED(status)) ! e = WEXITSTATUS(status); ! else if (WIFSIGNALED(status)) ! e = WTERMSIG(status); ! else /* can't happen */ ! e = 1; ! } ! if (die) done(e); } --- 220,235 ---- { pid_t pid; ! int e, status; ! e = 0; ! wait(&status); ! if (WIFEXITED(status)) ! e = WEXITSTATUS(status); ! else if (WIFSIGNALED(status)) ! e = WTERMSIG(status); ! else /* can't happen */ ! e = 1; ! if (e) done(e); } *************** *** 264,267 **** --- 261,265 ---- { (void)kill(0, SIGTERM); + (void)close(master); done(1); } *************** *** 280,284 **** } (void)fclose(fscript); - (void)close(master); exit(eno); } --- 278,281 ---- >Release-Note: >Audit-Trail: >Unformatted: