lib/pty-session: make wait_child callback optional

Now the code is duplicate on many places, but all we usually need is to
remember child status. It seems good enough to have very simple
callback child_die() to inform application about a change.

The patch also add PID to all signal related callbacks.

Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/include/pty-session.h b/include/pty-session.h
index 272dbac04718ee412ada17235ec522eed83c11b4..2adad102ec17ff4df458a2928e4ad6a87f0afafb 100644 (file)
--- a/include/pty-session.h
+++ b/include/pty-session.h
@@ -19,15 +19,21 @@
  */
 struct ul_pty_callbacks {
        /*
-        * Executed on SIGCHLD when ssi_code is EXITED, KILLED or DUMPED
-        * @
+        * Optional. Executed on SIGCHLD when ssi_code is EXITED, KILLED or
+        * DUMPED; The callback has to call ul_pty_set_child(pty, (pid_t) -1)
+        * if child is no more alive.
         */
-       void (*child_wait)(void *);
+       void (*child_wait)(void *, pid_t);
+
+       /*
+        * Used when child_wait() undefined to informa about child status
+        */
+       void (*child_die)(void *, pid_t, int);
 
        /*
         * Executed on SIGCHLD when ssi_status is SIGSTOP
         */
-       void (*child_sigstop)(void *);
+       void (*child_sigstop)(void *, pid_t);
 
        /*
         * Executed in master loop before ul_pty enter poll() and in time set by
@@ -90,5 +96,7 @@ int ul_pty_proxy_master(struct ul_pty *pty);
 
 void ul_pty_set_mainloop_time(struct ul_pty *pty, struct timeval *tv);
 int ul_pty_get_childfd(struct ul_pty *pty);
+void ul_pty_wait_for_child(struct ul_pty *pty);
+pid_t ul_pty_get_child(struct ul_pty *pty);
 
 #endif /* UTIL_LINUX_PTY_H */

diff --git a/lib/pty-session.c b/lib/pty-session.c
index d91f2e821cdafb9b001ad01606e1b916e231906a..bf60e79468ca17da3523aa1eac02c3b2046a63d2 100644 (file)
--- a/lib/pty-session.c
+++ b/lib/pty-session.c
@@ -102,6 +102,12 @@ int ul_pty_get_childfd(struct ul_pty *pty)
        return pty->master;
 }
 
+pid_t ul_pty_get_child(struct ul_pty *pty)
+{
+       assert(pty);
+       return pty->child;
+}
+
 /* it's active when signals are redurected to sigfd */
 int ul_pty_is_running(struct ul_pty *pty)
 {
@@ -312,6 +318,45 @@ static int handle_io(struct ul_pty *pty, int fd, int *eof)
        return rc;
 }
 
+void ul_pty_wait_for_child(struct ul_pty *pty)
+{
+       int status;
+       pid_t pid;
+       int options = 0;
+
+       if (pty->child == (pid_t) -1)
+               return;
+
+       DBG(SIG, ul_debug("waiting for child"));
+
+       if (ul_pty_is_running(pty)) {
+               /* wait for specific child */
+               options = WNOHANG;
+               for (;;) {
+                       pid = waitpid(pty->child, &status, options);
+                       if (pid != (pid_t) - 1) {
+                               if (pty->callbacks.child_die)
+                                       pty->callbacks.child_die(
+                                                       pty->callback_data,
+                                                       pty->child, status);
+                               ul_pty_set_child(pty, (pid_t) -1);
+                       } else
+                               break;
+               }
+       } else {
+               /* final wait */
+               while ((pid = wait3(&status, options, NULL)) > 0) {
+                       if (pid == pty->child) {
+                               if (pty->callbacks.child_die)
+                                       pty->callbacks.child_die(
+                                                       pty->callback_data,
+                                                       pty->child, status);
+                               ul_pty_set_child(pty, (pid_t) -1);
+                       }
+               }
+       }
+}
+
 static int handle_signal(struct ul_pty *pty, int fd)
 {
        struct signalfd_siginfo info;
@@ -333,11 +378,17 @@ static int handle_signal(struct ul_pty *pty, int fd)
 
                if (info.ssi_code == CLD_EXITED
                    || info.ssi_code == CLD_KILLED
-                   || info.ssi_code == CLD_DUMPED)
-                       pty->callbacks.child_wait(pty->callback_data);
+                   || info.ssi_code == CLD_DUMPED) {
+
+                       if (pty->callbacks.child_wait)
+                               pty->callbacks.child_wait(pty->callback_data,
+                                                         pty->child);
+                       else
+                               ul_pty_wait_for_child(pty);
 
-               else if (info.ssi_status == SIGSTOP && pty->child > 0)
-                       pty->callbacks.child_sigstop(pty->callback_data);
+               } else if (info.ssi_status == SIGSTOP && pty->child > 0)
+                       pty->callbacks.child_sigstop(pty->callback_data,
+                                                    pty->child);
 
                if (pty->child <= 0) {
                        pty->poll_timeout = 10;
@@ -535,60 +586,22 @@ done:
  * ... and see for example tty(1) or "ps afu"
  */
 struct ptytest {
-       pid_t   child;
-       int     childstatus;
-
        struct ul_pty *pty;
 };
 
-/* on child exit/dump/... */
-static void wait_for_child(void *data)
-{
-       struct ptytest *ss = (struct ptytest *) data;
-       int status;
-       pid_t pid;
-       int options = 0;
-
-       if (ss->child == (pid_t) -1)
-               return;
-
-       if (ul_pty_is_running(ss->pty)) {
-               /* wait for specific child */
-               options = WNOHANG;
-               for (;;) {
-                       pid = waitpid(ss->child, &status, options);
-                       if (pid != (pid_t) - 1) {
-                               ss->childstatus = status;
-                               ss->child = (pid_t) -1;
-                               ul_pty_set_child(ss->pty, (pid_t) -1);
-                       } else
-                               break;
-               }
-       } else {
-               /* final wait */
-               while ((pid = wait3(&status, options, NULL)) > 0) {
-                       if (pid == ss->child) {
-                               ss->childstatus = status;
-                               ss->child = (pid_t) -1;
-                               ul_pty_set_child(ss->pty, (pid_t) -1);
-                       }
-               }
-       }
-}
-
-static void child_sigstop(void *data)
+static void child_sigstop(void *data __attribute__((__unused__)), pid_t child)
 {
-       struct ptytest *ss = (struct ptytest *) data;
        kill(getpid(), SIGSTOP);
-       kill(ss->child, SIGCONT);
+       kill(child, SIGCONT);
 }
 
 int main(int argc, char *argv[])
 {
-       struct ptytest ss = { .child = (pid_t) -1 };
+       struct ptytest ss = { .pty = NULL };
        struct ul_pty_callbacks *cb;
        const char *shell, *command = NULL, *shname = NULL;
        int caught_signal = 0;
+       pid_t child;
 
        shell = getenv("SHELL");
        if (shell == NULL)
@@ -604,7 +617,6 @@ int main(int argc, char *argv[])
 
        ul_pty_set_callback_data(ss.pty, (void *) &ss);
        cb = ul_pty_get_callbacks(ss.pty);
-       cb->child_wait = wait_for_child;
        cb->child_sigstop = child_sigstop;
 
        if (ul_pty_setup(ss.pty))
@@ -612,7 +624,7 @@ int main(int argc, char *argv[])
 
        fflush(stdout);                 /* ??? */
 
-       switch ((int) (ss.child = fork())) {
+       switch ((int) (child = fork())) {
        case -1: /* error */
                ul_pty_cleanup(ss.pty);
                err(EXIT_FAILURE, "cannot create child process");
@@ -638,7 +650,7 @@ int main(int argc, char *argv[])
        }
 
        /* parent */
-       ul_pty_set_child(ss.pty, ss.child);
+       ul_pty_set_child(ss.pty, child);
 
        /* this is the main loop */
        ul_pty_proxy_master(ss.pty);
@@ -646,18 +658,19 @@ int main(int argc, char *argv[])
        /* all done; cleanup and kill */
        caught_signal = ul_pty_get_delivered_signal(ss.pty);
 
-       if (!caught_signal && ss.child != (pid_t)-1)
-               wait_for_child(&ss);    /* final wait */
+       if (!caught_signal && ul_pty_get_child(ss.pty) != (pid_t)-1)
+               ul_pty_wait_for_child(ss.pty);  /* final wait */
 
-       if (caught_signal && ss.child != (pid_t)-1) {
+       if (caught_signal && ul_pty_get_child(ss.pty) != (pid_t)-1) {
                fprintf(stderr, "\nSession terminated, killing shell...");
-               kill(ss.child, SIGTERM);
+               kill(child, SIGTERM);
                sleep(2);
-               kill(ss.child, SIGKILL);
+               kill(child, SIGKILL);
                fprintf(stderr, " ...killed.\n");
        }
 
        ul_pty_cleanup(ss.pty);
+       ul_free_pty(ss.pty);
        return EXIT_SUCCESS;
 }
 

diff --git a/login-utils/su-common.c b/login-utils/su-common.c
index 1f4fc9777dd602dbabb296be69486596ccbdf275..93ca0c2c50417ebeaac0811b46ebe89f8c73b1e6 100644 (file)
--- a/login-utils/su-common.c
+++ b/login-utils/su-common.c
@@ -182,7 +182,7 @@ static void init_tty(struct su_context *su)
  */
 static int wait_for_child(struct su_context *su)
 {
-       pid_t pid = (pid_t) -1;;
+       pid_t pid = (pid_t) -1;
        int status = 0;
 
        if (su->child == (pid_t) -1)
@@ -243,7 +243,9 @@ static int wait_for_child(struct su_context *su)
 }
 
 #ifdef USE_PTY
-static void wait_for_child_cb(void *data)
+static void wait_for_child_cb(
+                       void *data,
+                       pid_t child __attribute__((__unused__)))
 {
        wait_for_child((struct su_context *) data);
 }

diff --git a/term-utils/script.c b/term-utils/script.c
index 00b8d36b9c8d1bc7a16a8b447438e0297711f978..cc6c1790f28ba3656d94ed50059954beeb79a805 100644 (file)
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -82,10 +82,6 @@ UL_DEBUG_DEFINE_MASKNAMES(script) = UL_DEBUG_EMPTY_MASKNAMES;
 #define DBG(m, x)       __UL_DBG(script, SCRIPT_DEBUG_, m, x)
 #define ON_DBG(m, x)    __UL_DBG_CALL(script, SCRIPT_DEBUG_, m, x)
 
-#if defined(HAVE_LIBUTIL) && defined(HAVE_PTY_H)
-# include <pty.h>
-#endif
-
 #ifdef HAVE_LIBUTEMPTER
 # include <utempter.h>
 #endif
@@ -138,7 +134,7 @@ struct script_control {
        int ttycols;
        int ttylines;
 
-       struct ul_pty *pty;
+       struct ul_pty *pty;     /* pseudo-terminal */
        pid_t child;            /* child pid */
        int childstatus;        /* child process exit value */
 
@@ -582,50 +578,25 @@ static void log_done(struct script_control *ctl, const char *msg)
                log_close(ctl, ctl->in.logs[i], msg, status);
 }
 
-static void wait_for_child(void *data)
+static void callback_child_die(
+                       void *data,
+                       pid_t child __attribute__((__unused__)),
+                       int status)
 {
        struct script_control *ctl = (struct script_control *) data;
-       int status;
-       pid_t pid;
-       int options = 0;
-
-       if (ctl->child == (pid_t) -1)
-               return;
 
-       DBG(MISC, ul_debug("waiting for child"));
-
-       if (ul_pty_is_running(ctl->pty)) {
-               /* wait for specific child */
-               options = WNOHANG;
-               for (;;) {
-                       pid = waitpid(ctl->child, &status, options);
-                       if (pid != (pid_t) - 1) {
-                               ctl->childstatus = status;
-                               ctl->child = (pid_t) -1;
-                               ul_pty_set_child(ctl->pty, (pid_t) -1);
-                       } else
-                               break;
-               }
-       } else {
-               /* final wait */
-               while ((pid = wait3(&status, options, NULL)) > 0) {
-                       if (pid == ctl->child) {
-                               ctl->childstatus = status;
-                               ctl->child = (pid_t) -1;
-                               ul_pty_set_child(ctl->pty, (pid_t) -1);
-                       }
-               }
-       }
+       ctl->child = (pid_t) -1;
+       ctl->childstatus = status;
 }
 
-static void callback_child_sigstop(void *data)
+static void callback_child_sigstop(
+                       void *data __attribute__((__unused__)),
+                       pid_t child)
 {
-       struct script_control *ctl = (struct script_control *) data;
-
        DBG(SIGNAL, ul_debug(" child stop by SIGSTOP -- stop parent too"));
        kill(getpid(), SIGSTOP);
        DBG(SIGNAL, ul_debug(" resume"));
-       kill(ctl->child, SIGCONT);
+       kill(child, SIGCONT);
 }
 
 static int callback_log_stream_activity(void *data, int fd, char *buf, size_t bufsz)
@@ -868,7 +839,7 @@ int main(int argc, char **argv)
 
        ul_pty_set_callback_data(ctl.pty, (void *) &ctl);
        cb = ul_pty_get_callbacks(ctl.pty);
-       cb->child_wait = wait_for_child;
+       cb->child_die = callback_child_die;
        cb->child_sigstop = callback_child_sigstop;
        cb->log_stream_activity = callback_log_stream_activity;
        cb->log_signal = callback_log_signal;
@@ -932,6 +903,7 @@ int main(int argc, char **argv)
        if (rc)
                goto done;
 
+       /* add extra info to advanced timing file */
        if (timingfile && format == SCRIPT_FMT_TIMING_MULTI) {
                char buf[FORMAT_TIMESTAMP_MAX];
                time_t tvec = script_time((time_t *)NULL);
@@ -963,7 +935,7 @@ int main(int argc, char **argv)
        caught_signal = ul_pty_get_delivered_signal(ctl.pty);
 
        if (!caught_signal && ctl.child != (pid_t)-1)
-               wait_for_child(&ctl);   /* final wait */
+               ul_pty_wait_for_child(ctl.pty); /* final wait */
 
        if (caught_signal && ctl.child != (pid_t)-1) {
                fprintf(stderr, "\nSession terminated, killing shell...");

diff --git a/term-utils/scriptlive.c b/term-utils/scriptlive.c
index c7a25ae8924d39a3b47d1c5a3c2d9f604f2448b8..d229e6d6fa49408cc376f0a75ce4a39f6dfc0150 100644 (file)
--- a/term-utils/scriptlive.c
+++ b/term-utils/scriptlive.c
@@ -94,46 +94,23 @@ getnum(const char *s)
        return d;
 }
 
-/* on child exit/dump/... */
-static void wait_for_child(void *data)
+static void callback_child_die(
+                       void *data,
+                       pid_t child __attribute__((__unused__)),
+                       int status)
 {
        struct scriptlive *ss = (struct scriptlive *) data;
-       int status;
-       pid_t pid;
-       int options = 0;
-
-       if (ss->child == (pid_t) -1)
-               return;
-
-       if (ul_pty_is_running(ss->pty)) {
-               /* wait for specific child */
-               options = WNOHANG;
-               for (;;) {
-                       pid = waitpid(ss->child, &status, options);
-                       if (pid != (pid_t) - 1) {
-                               ss->childstatus = status;
-                               ss->child = (pid_t) -1;
-                               ul_pty_set_child(ss->pty, (pid_t) -1);
-                       } else
-                               break;
-               }
-       } else {
-               /* final wait */
-               while ((pid = wait3(&status, options, NULL)) > 0) {
-                       if (pid == ss->child) {
-                               ss->childstatus = status;
-                               ss->child = (pid_t) -1;
-                               ul_pty_set_child(ss->pty, (pid_t) -1);
-                       }
-               }
-       }
+
+       ss->child = (pid_t) -1;
+       ss->childstatus = status;
 }
 
-static void child_sigstop(void *data)
+static void callback_child_sigstop(
+                       void *data __attribute__((__unused__)),
+                       pid_t child)
 {
-       struct scriptlive *ss = (struct scriptlive *) data;
        kill(getpid(), SIGSTOP);
-       kill(ss->child, SIGCONT);
+       kill(child, SIGCONT);
 }
 
 static int process_next_step(struct scriptlive *ss)
@@ -314,8 +291,8 @@ main(int argc, char *argv[])
 
        ul_pty_set_callback_data(ss.pty, (void *) &ss);
        cb = ul_pty_get_callbacks(ss.pty);
-       cb->child_wait = wait_for_child;
-       cb->child_sigstop = child_sigstop;
+       cb->child_die = callback_child_die;
+       cb->child_sigstop = callback_child_sigstop;
        cb->mainloop = mainloop_cb;
 
        if (ul_pty_setup(ss.pty))
@@ -365,7 +342,7 @@ main(int argc, char *argv[])
        caught_signal = ul_pty_get_delivered_signal(ss.pty);
 
        if (!caught_signal && ss.child != (pid_t)-1)
-               wait_for_child(&ss);    /* final wait */
+               ul_pty_wait_for_child(ss.pty);  /* final wait */
 
        if (caught_signal && ss.child != (pid_t)-1) {
                fprintf(stderr, _("\nSession terminated, killing shell..."));

diff --git a/term-utils/scriptreplay.c b/term-utils/scriptreplay.c
index 609b20f93e515dac7b4a99ed1f7ec48c6744b179..64c95df694ad813be7d0bb9283010d7063283b2d 100644 (file)
--- a/term-utils/scriptreplay.c
+++ b/term-utils/scriptreplay.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008, Karel Zak <kzak@redhat.com>
+ * Copyright (C) 2008-2019, Karel Zak <kzak@redhat.com>
  * Copyright (C) 2008, James Youngman <jay@gnu.org>
  *
  * This file is free software; you can redistribute it and/or modify