[PATCH 2/2] posix: Add terminal control setting support for posix_spawn

[Godmar Back]

Thank you for cc'ing me. I am assuming you're asking for feedback on your patch.

A few notes with what I've noticed.

(1)
+/* Returh the associated terminal FD in the attribute structure.  */

Should it be "Return?"

(2)
+  int __tcpgrp;

This field holds a file descriptor, but the name '__tcpgrp' could be
misinterpreted as a "pgrp" - as in "terminal control process group"
If would suggest a different name perhaps, such as '__ctty_fd' or
'__tcfd' or something like that to make it more clear that the field
contains a file descriptor.

(3)
Is passing '0600' necessary if you open with O_RDONLY as in:

+  int fd = xopen (_PATH_TTY, O_RDONLY, 0600);


(4)
I didn't see any tests, and I'm not sure the code would work as it's written.
Specifically, the child process needs to block (mask) SIGTTOU around
the tcsetpgrp() call.
This is because after it calls `setpgid` to place itself into a new
process group, it will no longer be in the terminal's foreground
process group (by definition, all process groups that are not the
foreground process group are "background process groups").
Consequently, attempts to call `tcsetpgrp()` will result in SIGTTOU,
which must be blocked.

See tcgetpgrp(3):

   If tcsetpgrp() is called by a member of a background process group
in its session, and the calling process is not
   blocking or ignoring  SIGTTOU, a SIGTTOU signal is sent to all
members of this background process group.

Try the following test program to see this:

#include <unistd.h>
#include <fcntl.h>
#include <signal.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sys/wait.h>

int
main()
{
    pid_t pgid = 0;
    int ttyfd = open("/dev/tty", O_RDONLY);

    // typical sequence if user types 'a | b' (without the pipes)
    for (int i = 0; i < 2; i++) {
        int child = fork();
        if (child == 0) {
            setpgid(0, pgid);
            // you need this call
            // signal(SIGTTOU, SIG_IGN);
            int rc = tcsetpgrp(ttyfd, getpgrp());
            if (rc != 0) {
                perror("tcsetpgrp");
                abort();
            }
            printf("child process %d group %d successfully has
terminal ownership\n", getpid(), getpgrp());
            exit(0);
        } else {
            setpgid(child, pgid);
        }

        if (pgid == 0)
            pgid = child;

        printf("spawned #%d\n", i+1);
    }

    for (int i = 0; i < 2; i++) {
        int status;
        int child = waitpid(-1, &status, WUNTRACED);
        if (WIFSTOPPED(status)) {
            printf("child %d was stopped with %s\n", child,
strsignal(WSTOPSIG(status)));
        } else
        if (WIFEXITED(status)) {
            printf("child %d exited with %d\n", child, WEXITSTATUS(status));
        }
    }
}

(5) Since you have to block (or ignore) SIGTTOU around the call to
tcsetpgrp, you would have to possibly restore the original signal
state before exec'ing the child.
This may interact with POSIX_SPAWN_SETSIGMASK and/or
POSIX_SPAWN_SETSIGDEF if the caller to posix_spawn() includes SIGTTOU
there. In either case, the correct semantics would have to be
implemented.