Instead of using the _child_reader singleton to track the child process,
use variables on the stack. Also, limit the variable scope as much as
possible and used named constants for the pipe indices.

Signed-off-by: Joshua Watt <jpewhac...@gmail.com>
---
 utils.c | 108 +++++++++++++++++++++++++-------------------------------
 1 file changed, 48 insertions(+), 60 deletions(-)

diff --git a/utils.c b/utils.c
index aacf123..bd52544 100644
--- a/utils.c
+++ b/utils.c
@@ -55,14 +55,10 @@
 
 #define UNUSED(x) (void)(x)
 
-static struct {
-       int fds[2];
-       FILE *fps[2];
-
-       unsigned int timeout;
-       int timeouted;
-       int padding1;
-} _child_reader;
+enum {
+       PIPE_READ = 0,
+       PIPE_WRITE = 1,
+};
 
 static inline char *
 get_stime(char *stime, size_t size, time_t t)
@@ -398,15 +394,7 @@ run_ptests(struct ptest_list *head, const struct 
ptest_options opts,
        FILE *xh = NULL;
 
        struct ptest_list *p;
-       char stime[GET_STIME_BUF_SIZE];
 
-       pid_t child;
-       int pipefd_stdout[2] = {-1, -1};
-       int pipefd_stderr[2] = {-1, -1};
-       time_t sttime, entime;
-       time_t duration;
-       int slave;
-       int pgid = -1;
 
        if (opts.xml_filename) {
                xh = xml_create(ptest_list_length(head), opts.xml_filename);
@@ -423,12 +411,16 @@ run_ptests(struct ptest_list *head, const struct 
ptest_options opts,
 
                fprintf(fp, "START: %s\n", progname);
                PTEST_LIST_ITERATE_START(head, p)
+                       int pipefd_stdout[2] = {-1, -1};
+                       int pipefd_stderr[2] = {-1, -1};
+                       int pgid = -1;
+
                        if ((rc = pipe2(pipefd_stdout, 0)) == -1)
                                break;
 
                        if ((rc = pipe2(pipefd_stderr, 0)) == -1) {
-                               close(pipefd_stdout[0]);
-                               close(pipefd_stdout[1]);
+                               close(pipefd_stdout[PIPE_READ]);
+                               close(pipefd_stdout[PIPE_WRITE]);
                                break;
                        }
 
@@ -443,16 +435,18 @@ run_ptests(struct ptest_list *head, const struct 
ptest_options opts,
                                fprintf(fp, "ERROR: getpgid() failed, %s\n", 
strerror(errno));
                        }
 
-                       child = fork();
+                       pid_t child = fork();
                        if (child == -1) {
                                fprintf(fp, "ERROR: Fork %s\n", 
strerror(errno));
                                rc = -1;
                                break;
                        } else if (child == 0) {
+                               int slave;
+
                                close(0);
                                /* Close read ends of the pipe */
-                               do_close(&pipefd_stdout[0]);
-                               do_close(&pipefd_stderr[0]);
+                               do_close(&pipefd_stdout[PIPE_READ]);
+                               do_close(&pipefd_stderr[PIPE_READ]);
 
                                if ((slave = setup_slave_pty(fp)) < 0) {
                                        fprintf(fp, "ERROR: could not setup pty 
(%d).", slave);
@@ -469,37 +463,35 @@ run_ptests(struct ptest_list *head, const struct 
ptest_options opts,
                                        fprintf(fp, "ERROR: Unable to attach to 
controlling tty, %s\n", strerror(errno));
                                }
 
-                               run_child(p->run_ptest, pipefd_stdout[1], 
pipefd_stderr[1]);
+                               run_child(p->run_ptest, 
pipefd_stdout[PIPE_WRITE], pipefd_stderr[PIPE_WRITE]);
 
                        } else {
-                               int status;
+                               bool timedout = false;
+                               char stime[GET_STIME_BUF_SIZE];
 
                                /* Close write ends of the pipe, otherwise this 
process will never get EOF when the child dies */
-                               do_close(&pipefd_stdout[1]);
-                               do_close(&pipefd_stderr[1]);
-
-                               _child_reader.fds[0] = pipefd_stdout[0];
-                               _child_reader.fds[1] = pipefd_stderr[0];
-                               _child_reader.fps[0] = fp;
-                               _child_reader.fps[1] = fp_stderr;
-                               _child_reader.timeout = opts.timeout;
-                               _child_reader.timeouted = 0;
+                               do_close(&pipefd_stdout[PIPE_WRITE]);
+                               do_close(&pipefd_stderr[PIPE_WRITE]);
+
                                if (setpgid(child, pgid) == -1) {
                                        fprintf(fp, "ERROR: setpgid() failed, 
%s\n", strerror(errno));
                                }
 
-                               sttime = time(NULL);
-                               fprintf(fp, "%s\n", get_stime(stime, 
GET_STIME_BUF_SIZE, sttime));
+                               time_t start_time= time(NULL);
+                               fprintf(fp, "%s\n", get_stime(stime, 
GET_STIME_BUF_SIZE, start_time));
                                fprintf(fp, "BEGIN: %s\n", ptest_dir);
 
-                               set_nonblocking(_child_reader.fds[0]);
-                               set_nonblocking(_child_reader.fds[1]);
-
                                struct pollfd pfds[2];
-                               for (int i = 0; i < 2; i++) {
-                                       pfds[i].fd = _child_reader.fds[i];
-                                       pfds[i].events = POLLIN;
-                               }
+                               FILE* dest_fps[2];
+                               set_nonblocking(pipefd_stdout[PIPE_READ]);
+                               pfds[0].fd = pipefd_stdout[PIPE_READ];
+                               pfds[0].events = POLLIN;
+                               dest_fps[0] = fp;
+
+                               set_nonblocking(pipefd_stderr[PIPE_READ]);
+                               pfds[1].fd = pipefd_stderr[PIPE_READ];
+                               pfds[1].events = POLLIN;
+                               dest_fps[1] = fp_stderr;
 
                                while (true) {
                                        /*
@@ -520,9 +512,9 @@ run_ptests(struct ptest_list *head, const struct 
ptest_options opts,
                                                break;
                                        }
 
-                                       int ret = poll(pfds, 2, 
_child_reader.timeout*1000);
+                                       int ret = poll(pfds, 2, 
opts.timeout*1000);
 
-                                       if (ret == 0 && 
!_child_reader.timeouted) {
+                                       if (ret == 0 && !timedout) {
                                                /* kill the child if we haven't
                                                 * already. Note that we
                                                 * continue to read data from
@@ -530,7 +522,7 @@ run_ptests(struct ptest_list *head, const struct 
ptest_options opts,
                                                 * sure we get all the output
                                                 */
                                                kill(-child, SIGKILL);
-                                               _child_reader.timeouted = 1;
+                                               timedout = true;
                                        }
 
                                        for (int i = 0; i < 2; i++) {
@@ -551,13 +543,13 @@ run_ptests(struct ptest_list *head, const struct 
ptest_options opts,
                                                                }
                                                                continue;
                                                        } else {
-                                                               fwrite(buf, 
(size_t)n, 1, _child_reader.fps[i]);
+                                                               fwrite(buf, 
(size_t)n, 1, dest_fps[i]);
                                                        }
                                                }
                                        }
                                }
 
-                               if (_child_reader.timeouted) {
+                               if (timedout) {
                                        collect_system_state(fp);
                                } else {
                                        /*
@@ -570,10 +562,11 @@ run_ptests(struct ptest_list *head, const struct 
ptest_options opts,
                                         */
                                        kill(-child, SIGKILL);
                                }
+                               int status;
                                waitpid(child, &status, 0);
 
-                               entime = time(NULL);
-                               duration = entime - sttime;
+                               time_t end_time = time(NULL);
+                               time_t duration = end_time - start_time;
 
                                int exit_code = -1;
                                if (WIFEXITED(status)) {
@@ -591,30 +584,25 @@ run_ptests(struct ptest_list *head, const struct 
ptest_options opts,
                                        rc += 1;
                                }
                                fprintf(fp, "DURATION: %d\n", (int) duration);
-                               if (_child_reader.timeouted) {
+                               if (timedout) {
                                        fprintf(fp, "TIMEOUT: %s\n", ptest_dir);
                                        rc += 1;
                                }
 
                                if (opts.xml_filename)
-                                       xml_add_case(xh, exit_code, ptest_dir, 
_child_reader.timeouted, (int) duration);
+                                       xml_add_case(xh, exit_code, ptest_dir, 
timedout, (int) duration);
 
                                fprintf(fp, "END: %s\n", ptest_dir);
-                               fprintf(fp, "%s\n", get_stime(stime, 
GET_STIME_BUF_SIZE, entime));
+                               fprintf(fp, "%s\n", get_stime(stime, 
GET_STIME_BUF_SIZE, end_time));
                        }
                        free(ptest_dir);
-                       do_close(&pipefd_stdout[0]);
-                       do_close(&pipefd_stdout[1]);
-                       do_close(&pipefd_stderr[0]);
-                       do_close(&pipefd_stderr[1]);
+                       do_close(&pipefd_stdout[PIPE_READ]);
+                       do_close(&pipefd_stdout[PIPE_WRITE]);
+                       do_close(&pipefd_stderr[PIPE_READ]);
+                       do_close(&pipefd_stderr[PIPE_WRITE]);
 
                PTEST_LIST_ITERATE_END
                fprintf(fp, "STOP: %s\n", progname);
-
-               do_close(&pipefd_stdout[0]);
-               do_close(&pipefd_stdout[1]);
-               do_close(&pipefd_stderr[0]);
-               do_close(&pipefd_stderr[1]);
        } while (0);
 
        if (rc == -1)
-- 
2.33.0

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#60609): https://lists.yoctoproject.org/g/yocto/message/60609
Mute This Topic: https://lists.yoctoproject.org/mt/100220153/21656
Group Owner: yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to