BrokenStarz111
BrokenStarz111

Reputation: 41

How do I use 2 child processes one for executing command and the other one for reading output and passing it to the next?

So my program needs to pipe multiple processes and read the number of bytes each process output has.

The way I implemented it, in a for loop, we have two children:

Child 1: dups output and executes the process

Child 2: reads the output and writes it for the next input

Currently, child 1 executes the process and the child 2 reads its output, but it doesn't seem to write it in the right place because in the second loop iteration it prints the output to the screen and blocks.

for (int i = 0; i < processes; i++) {
      
    int result = socketpair(PF_LOCAL, SOCK_STREAM, 0, apipe[i]);

    if (result == -1) {

      error_and_exit();

    }

    int pid;
    int pid2;

    pid = fork_or_die();
    // child one points to STDOUT

    if (pid == FORK_CHILD) {

        if (dup2(apipe[i][1], STDOUT_FILENO) == -1)
            error_and_exit();

        if (close(apipe[i][1]) == -1)
            error_and_exit();
           
        if (close(apipe[i][0]) == -1)
            error_and_exit();  

        if (execlp("/bin/sh", "sh", "-c", tabCommande[i], (char *)NULL) == -1)
            error_and_exit();    
    }

    pid2 = fork_or_die();

    //CHILD 2 reads the output and writes if for the next command to use

    if(pid2 == FORK_CHILD){      
        FILE *fp;

        fp = fopen("count", "a");

        close(apipe[i][1]);
        int count=0;

        char str[4096];
        count = read(apipe[i][0], str, sizeof(str)+1);
        close(apipe[i][0]);

        write(STDIN_FILENO, str, count);

        fprintf(fp, "%d : %d \n ", i, count);  
        fclose(fp);

     }
}

Upvotes: 0

Views: 288

Answers (1)

Jonathan Leffler
Jonathan Leffler

Reputation: 754900

Your second child does “write(STDIN_FILENO, …); that’s not a conventional way of using standard input.

If standard input is a terminal, then the device is usually opened for reading and writing and the three standard I/O channels are created using dup() or dup2(). Thus you can read from the outputs and write to the input — but only if the streams are connected to a login terminal (window). If the input is a pipe, you can't successfully write to it, nor can you read from the output if it is a pipe. (Similarly if the input is redirected from a file or the output is redirected to a file.) This terminal setup is done by the process that creates the terminal window. It is background information explaining why writing to standard input appears on the terminal.

Anyway, that's what you're doing. You are writing to the terminal via standard input. Your minimum necessary change is to replace STDIN_FILENO with STDOUT_FILENO.

You are also going to need a loop around the reading and writing code. In general, processes generate lots of output in small chunks. The close on the input pipe will be outside the loop, of course, not between the read() and write() operations. You should check that the write() operations write all the data to the output.

You should also have the second child exit after it closes the output file. In this code, I'd probably open the file after the counting loop (or what will become the counting loop), but that's mostly a stylistic change, keeping the scope of variables to a minimum.

You will probably eventually need to handle signals like SIGPIPE (or ignore it so that the output functions return errors when the pipe is closed early). However, that's a refinement for when you have the basic code working.

Bug: you have:

count = read(apipe[i][0], str, sizeof(str)+1);

This is a request to the o/s to give you a buffer overflow — you ask it to write more data into str than str can hold. Remove the +1!

Minor note: you don’t need to check the return value from execlp() or any of that family of functions. If the call succeeds, it doesn’t return; if it returns, it failed. Your code is correct to exit after the call to execlp(), though; that's good.


You said:

I replaced STDIN_FILENO to STDOUT_FILENO in the second child but it doesn't seem to solve the issue. The output is still shown in the terminal and there's a pipe blockage after.

That observation may well be correct, but it isn't something that can be resolved by studying this code alone. The change to write to an output stream is necessary — and in the absence of any alternative information, writing to STDOUT_FILENO is better than writing to STDIN_FILENO.

That is a necessary change, but it is probably not a sufficient change. There are other changes needed too.

Did you set up the inputs and outputs for the pair of children this code creates correctly? It is very hard to know from the code shown — but given that it is not working as you intended, it's a reasonable inference that you did not get all the plumbing correct. You need to draw a diagram of how the processes are intended to operate in the larger context. At a minimum, you need to know where the standard input for each process comes from, and where its standard input goes. Sometimes, you need to worry about standard error too — most likely though, in this case, you can quietly ignore it.


This is what I think your code could look like — though the comments in it describe numerous possible variants.

#include <sys/socket.h>
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>

/* The code needs these declarations and definition to compile */
extern _Noreturn void error_and_exit(void);
extern pid_t fork_or_die(void);
extern void unknown_function(void);

static ssize_t copy_bytes(int fd1, int fd2);

#define FORK_CHILD 0

int processes;
int apipe[20][2];
char *tabCommande[21];

void unknown_function(void)
{
    for (int i = 0; i < processes; i++)
    {
        int result = socketpair(PF_LOCAL, SOCK_STREAM, 0, apipe[i]);

        if (result == -1)
            error_and_exit();

        int pid1 = fork_or_die();
        // child one points to STDOUT

        if (pid1 == FORK_CHILD)
        {
            if (dup2(apipe[i][1], STDOUT_FILENO) == -1)
                error_and_exit();
            if (close(apipe[i][1]) == -1)
                error_and_exit();
            if (close(apipe[i][0]) == -1)
                error_and_exit();
            execlp("/bin/sh", "sh", "-c", tabCommande[i], (char *)NULL);
            error_and_exit();
        }

        //CHILD 2 reads the output and writes if for the next command to use
        int pid2 = fork_or_die();
        if (pid2 == FORK_CHILD)
        {
            close(apipe[i][1]);
            ssize_t count = copy_bytes(apipe[i][0], STDOUT_FILENO);
            FILE *fp = fopen("count", "a");
            if (fp == NULL)
                error_and_exit();
            /*
            ** Using %zd for ssize_t is a reasonable guess at a format to
            ** print ssize_t - but it is a guess.  Alternatively, change the
            ** type of count to long long and use %lld.  There isn't a
            ** documented, official (fully standardized by POSIX) conversion
            ** specifier for ssize_t AFAIK.
            */
            fprintf(fp, "%d : %zd\n ", i, count);
            fclose(fp);
            exit(EXIT_SUCCESS);
        }

        /*
        ** This is crucial - the parent has all the pipes open, and the
        ** child processes won't get EOF until the parent closes the
        ** write ends of the pipes, and they won't get EOF on the inputs
        ** until the parent closes the read ends of the pipe.
        **
        ** It could be avoided if the first child creates the pipe or
        ** socketpair and then creates the second child as a grandchild
        ** of the main process.  That also alters the process structure
        ** and reduces the number of processes that the original parent
        ** process has to wait for.  If the first child creates the
        ** pipe, then the apipe array of arrays becomes unnecessary;
        ** you can have a simple int apipe[2]; array that's local to the
        ** two processes.  However, you may need the array of arrays so
        ** that you can chain the outputs of one process (pair of
        ** processes) to the input of the next.
        */
        close(apipe[i][0]);
        close(apipe[i][1]);
    }
}

static ssize_t copy_bytes(int fd1, int fd2)
{
    ssize_t tbytes = 0;
    ssize_t rbytes;
    char buffer[4096];

    while ((rbytes = read(fd1, buffer, sizeof(buffer))) > 0)
    {
        ssize_t wbytes = write(fd2, buffer, rbytes);
        if (wbytes != rbytes)
        {
            /*
            ** There are many possible ways to deal with this.  If
            ** wbytes is negative, then the write failed, presumably
            ** irrecoverably.  The code could break the loop, reporting
            ** how many bytes were written successfully to the output.
            ** If wbytes is zero (pretty improbable), it isn't clear
            ** what happened.  If wbytes is positive, then you could add
            ** the current value to tbytes and try to write the rest in
            ** a loop until everything has been written or an error
            ** occurs.  You pays your money and takes your pick.
            */
            error_and_exit();
        }
        tbytes += wbytes;
    }

    if (tbytes == 0 && rbytes < 0)
        tbytes = rbytes;
    return tbytes;
}

You could add #include <signal.h> and signal(SIGPIPE, SIG_IGN); to the code in the second child.

Upvotes: 0

Related Questions