new applet: script
Pascal Bellard
pascal.bellard at ads-lu.com
Tue Feb 26 22:35:01 UTC 2008
A new version. Simpler and ... less buggy !
> On Sunday 24 February 2008 23:58, Pascal Bellard wrote:
>> Here is the updated patch.
>
>> > static void finish(int sig)
>> > How about less cryptic one ........?
>>
>> Maybe I'm wrong, but I want to catch all pending signals.
>
> I mean: other people will read your code. Many of them
> are less talented than you. Will they grok it correctly?
> How easy it will be to misunderstand it? So far, I feel like
> I too don't fully understand it. That's why I'm asking.
>
> Let's see. My thinking process:
>
> fail() does "kill(0, SIGTERM);". No TERM handler installed.
> Usually programs have it set to SIG_DFL (terminate process)
> by parent.
> So, fail just prints a message and kills itself? Why not
> just exit() instead of kill? Do you see some scenario
> where SIGTERM is set to SIG_IGN by our program's parent?
> Or what? I am confused! You have to explain it in comment.
>
> Ok, let's assume SIGTERM is set to SIG_IGN.
>
> done() always exits, restores term attrs from "tt" if !subchild.
Now only the parent restores term attr.
> fail() always calls done().
No more fail()
> finish _may_ call done().
>
> main program:
>
> pty = getpty(line,sizeof(line));
> if (pty < 0) {
> fail("Out of pty's"); <=== -> done(), will restore term
> attrs...
> ...but attrs are not saved yet to tt! Bug?
Yes, it is.
> }
> tcgetattr(0, &tt);
>
>
> child = fork();
> if (child) { /* parent: link mainshell stdin to pty input */
> close(fdscript);
> bb_copyfd_eof(0, pty);
> done(); <========= will restore term attrs
> }
>
> Suggestion: just add done() as a last statement, don't
> nest else's. They are confusing. [Undoing nesting here:]
Ok. Anyway the optimizer will move them
>
> /* child */
> subchild = child = fork();
> if (subchild < 0) {
> fail("fork"); <========= will NOT restore term attrs
> so, bb_perror_msg_and_exit("fork") is the same?
> }
> if (child) {
> /* child1: link pty output to mainshell stdout and file */
> char buf[256];
> close(0);
> while ((count = read(pty, buf, sizeof(buf))) > 0) {
> write(1, buf, count);
> write(fdscript, buf, count);
> if (fflg) {
> fsync(fdscript);
> }
> }
> done(); <========= will NOT restore term attrs
> so, why not just exit(0)? It would be clearer...
> }
> /* child2: link subshell input, output, error to pty */
> close(pty);
> close(fdscript);
> pty = xopen(line, O_RDWR);
> tcsetattr(pty, TCSAFLUSH, &tt);
> ioctl(pty, TIOCSWINSZ, (char *)&win);
> setsid();
> ioctl(pty, TIOCSCTTY, 0);
> xmove_fd(pty, 0);
> dup2(0, 1);
> dup2(0, 2);
> execl(shell, strrchr(shell, '/') + 1, shell_arg, cflg, NULL);
> fail(shell); /* -> done() */ <============ will restore term
> attrs?! (subshell == 0)
> Is it correct or a bug?
> If it is correct, add a comment and explain why.
Bug again. execl may not return.
>
> } /* end of main */
>
> So, after all this, it looks like fail() can be replaced by
> bb_[p]error_msg_and_exit(), and done with either exit()
> or tcsetattr() + exit(), as needed.
>
> Also, variable "child" is ok - it means "if !0, I have a child with this
> pid",
> but variable "subchild" is misnamed: 0 in parent, !0 in child1, and 0
> again
> in child2? It means "if !0, I am child1"? Maybe rename it then?
>
No more subchild. Parent instead.
>
> Similarly comfusing situation is with finish():
>
> /* Maybe sigchld_handler would be a better name? */
> static void finish(int sig)
> {
> int pid, die = 0;
>
> (void) sig;
> /* purge all waiting signals */
> while ((pid = wait_any_nohang(&sig)) > 0) {
> if (pid == child) {
> die = 1;
> }
> }
> if (die) {
> done();
> }
> }
>
> I do not understand. What signals? You mean "wait for all *children*,
> [not *signals*], and if we are parent and see child1 exiting,
> or if we are child1 and see child2 exiting [crap. I am becoming confused
> what is a child to what. This is a bad sign], we exit. If we are parent,
> we also restore term attrs. [Oh no. I'm wrong. We do it if
> we are parent _or_ child2? Why?]
>
> Question: how parent can ever see _anyone else_ exiting except child1?
> Or how child1 can ever see _anyone else_ exiting except child2? They have
> no other children! And grandchildren are not reparented to grandparent
> in UNIX, they go straight to PID 1, so it doesn't matter what
> grandchildren
> will be ever created by execl(shell, ...). So, if you ever see wait
> returning
> pid > 0, it is _your child you are waiting for_.
>
> Even better: if you got a SIGCHLD, you may be fairly sure it's THAT
> child exited. In the handler, just wait(NOHANG) in the loop until
> it returns < 0, then exit.
>
> Even better #2: both parent and child1 have loops a-la
> while(read(...)) write(...)
> They will stop looping as soon as there is nothing to read -
> that is, when child closed stdout/stderr.
> This is not the same as above, but close. It's in fact better -
> otherwise shell's grandchildren can outlive shell and continue
> (trying to) produce output, you probably want to catch that too.
with parent, stdin will never close.
>
> And in this case, you dont even NEED to catch SIGCHLD!
>
> POSIX.1-1990 disallowed setting the action for SIGCHLD to SIG_IGN.
> POSIX.1-2001 allows this possibility, so that
> ignoring SIGCHLD can be used to prevent the creation of zombies
> (see wait(2)). Nevertheless, the historical BSD and
> System V behaviours for ignoring SIGCHLD differ, so that the only
> completely portable method of ensuring that termi-
> nated children do not become zombies is to catch the SIGCHLD signal
> and perform a wait(2) or similar.
>
> I'd say set SIGCHLD to SIG_IGN in this case.
> --
> vda
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: busybox-20080226-APPLET-SCRIPT.u
Type: application/octet-stream
Size: 10039 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20080226/42de8273/attachment-0002.obj
More information about the busybox
mailing list