>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
[email protected]:/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:
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "[email protected]"
