Using pipes to communicate between two programs

Question

I need the main prog to get two strings from the user and an argument for the other program, call fork() and then in child process I need to write the strings into pipe and send them to the other program which returns an int which I want to pass to parent so I'm trying to use another pipe for it but every time it stops right after inserting the strings.

So the main program: (EDITED)

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

#define LINELEN (80)

char *mygets(char *buf, int len);
int mygeti();

int main(int argc, char *argv[])
{
    char *cmpstr[] = {"lexcmp", "lencmp"};
    int veclen = sizeof(cmpstr)/sizeof(char *);
    char str1[LINELEN + 1];
    char str2[LINELEN + 1];
    int index;
    int pid[2];
    int pfd[4][2];
    
    for(int i = 0; i < 4; i++)
    {
        if(pipe(pfd[i]) < 0)
        {
            perror("pipe");
            return -2;
        }
    }
    
    pid[0] = fork();
    
    if(pid[0] == 0) // child a
    {
        close(pfd[0][1]);
        close(pfd[2][0]);
        
        dup2(pfd[0][0], STDIN_FILENO);
        dup2(pfd[2][1], STDOUT_FILENO);

        char *myargs[3];
        myargs[0] = "./loopcmp";
        myargs[1] = "lexcmp";
        myargs[2] = NULL;
        if(execvp(myargs[0], myargs) == -1)
        {
            perror("exec");
            return -2;
        }
        close(pfd[0][0]);
        close(pfd[2][1]);
    }
    else
    {
        pid[1] = fork();
        if(pid[1] == 0) //child b
        {
            close(pfd[1][1]);
            close(pfd[3][0]);
            
            dup2(pfd[1][0], STDIN_FILENO);
            dup2(pfd[3][1], STDOUT_FILENO);
            
            char *myargs[3];
            myargs[0] = "./loopcmp";
            myargs[1] = "lencmp";
            myargs[2] = NULL;
            if(execvp(myargs[0], myargs) == -1)
            {
                perror("exec");
                return -2;
            }
            close(pfd[1][0]);
            close(pfd[3][1]);
        }
        else // parent
        {
            while (1)
            {
                printf("Please enter first string:\n");
                if (mygets(str1, LINELEN) == NULL)
                    break;
                printf("Please enter second string:\n");
                if (mygets(str2, LINELEN) == NULL)
                    break;
                do {
                    printf("Please choose:\n");
                    for (int i=0 ; i < veclen ; i++)
                        printf("%d - %s\n", i, cmpstr[i]);
                    index = mygeti();
                } while ((index < 0) || (index >= veclen));
                
                close(pfd[index][0]);

                if(write(pfd[index][1], str1, strlen(str1)) == -1)
                {
                    perror("writeToPipe");
                    return -2;
                }
                if(write(pfd[index][1], str2, strlen(str2)) == -1)
                {
                    perror("writeToPipe");
                    return -2;
                }
                
                if(index == 0)
                {
                    close(pfd[2][1]);
                    char rbuf[1];                    
                    while(read(pfd[2][0], &rbuf, 1) > 0)
                    {
                        write(STDOUT_FILENO, &rbuf, 1);
                    }
                }
                
                if(index == 1)
                {
                    close(pfd[3][1]);
                    char rbuf[1];                    
                    while(read(pfd[3][0], &rbuf, 1) > 0)
                    {
                        write(STDOUT_FILENO, &rbuf, 1);
                    }
                }
            }
        }
    }
    
    
    return 0;
}

char *mygets(char *buf, int len)
{
    char *retval;

    retval = fgets(buf, len, stdin);
    buf[len] = '\0';
    if (buf[strlen(buf) - 1] == 10) /* trim \r */
        buf[strlen(buf) - 1] = '\0';
    else if (retval) 
        while (getchar() != '\n'); /* get to eol */

    return retval;
}

int mygeti()
{
    int ch;
    int retval=0;

    while(isspace(ch=getchar()));
    while(isdigit(ch))
    {
        retval = retval * 10 + ch - '0';
        ch = getchar();
    }
    while (ch != '\n')
        ch = getchar();
    return retval;
}

The other program - loopcmp: (Here I shouldn't change anything)

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define LINELEN (80)

int lencmp(const char *str1, const char *str2);
int lexcmp(const char *str1, const char *str2);
char *mygets(char *buf, int len);

int main(int argc, char *argv[])
{
    int(*cmpfunc)(const char *, const char *) = NULL;
    char str1[LINELEN + 1];
    char str2[LINELEN + 1];

    if (argc != 2)
        return -1;
    
    if (!strcmp(argv[1], "lexcmp"))
        cmpfunc = lexcmp;
    else if (!strcmp(argv[1], "lencmp"))
        cmpfunc = lencmp;
    else
        return -1;

    while (1)
    {
        if (mygets(str1, LINELEN) == NULL)
            break;
        if (mygets(str2, LINELEN) == NULL)
            break;
        printf("%d\n", cmpfunc(str1, str2));
        fflush(stdout);
    }
    return 0;
}

int lencmp(const char *str1, const char *str2)
{
    int val;
    val = strlen(str1) - strlen(str2);
    if (val < 0)
        return 1;
    if (val > 0)
        return 2;
    return 0;
}

int lexcmp(const char *str1, const char *str2)
{
    int val;

    val = strcmp(str1, str2);
    if (val < 0)
        return 1;
    if (val > 0)
        return 2;
    return 0;
}

char *mygets(char *buf, int len)
{
    char *retval;

    retval = fgets(buf, len, stdin);
    buf[len] = '\0';
    if (buf[strlen(buf) - 1] == 10) /* trim \r */
        buf[strlen(buf) - 1] = '\0';
    else if (retval) while (getchar() != '\n'); /* get to eol */

    return retval;
}

This is what I get: Picture

and what I actually need it to print the interger returned from the exec of the child and then start again and get new two strings and so on till the user exits. what am I doing wrong? I can only modify the main program (the first one)

Answer 1

The first thing to do is ensure you are closing all unnecessary file descriptors in each process.

This means anything relating to the lexcmp child process should be closed in the lencmp child process, and vice versa. The parent needs to close the read ends of both "TO" pipes, and the write end of both "FROM" pipes.

Each of these closures should happen exactly once, where appropriate.

As is, in the parent, you are calling close(pfd[index][0]);, close(pfd[2][1]);, and close(pfd[3][1]); in a loop.

After calling dup2, you should immediately close the first argument (the original pipe end). As is, in the the children, you are attempting to close them after execvp is called, which leads into the next issue...

If execvp succeeds, it NEVER returns, as it will completely replace the process image. Anything expected to run after it is really operating in a failure state. So

if(execvp(myargs[0], myargs) == -1)
{
    perror("exec");
    return -2;
}

could be written as

execvp(myargs[0], myargs)
perror("exec");
return -2;

to the same effect.

---

Aside: the large if .. else if .. else structure of main is a bit hard to read, and not needed since the body of each if statement results in the child processes being replaced, or exiting on error.

---

The next issues have to do with deadlocking, which most typically occurs when two intercommunicating processes attempt blocking reads from one another at the same time.

Your child processes expect input in a very specific way: 2 lines at a time, creating a pair of strings. The two write calls, in the form of,

write(pfd[index][1], strX, strlen(strX))

do not write any newlines, thus the children wait forever, never to send any data back, and the parent will wait forever, never receiving any data.

---

Aside: mygets is severely flawed, in a few ways, including being unable to detect EOF or I/O failures (this function is a SIGSEGV in waiting). One of the more obnoxious failings is that the comment here

if (buf[strlen(buf) - 1] == 10) /* trim \r */

is just plain wrong. ASCII decimal 10 is '\n', the line feed, or newline character. '\r', or carriage return, would be decimal 13. This is why using character constants 'A' instead of integer constants 65 is highly encouraged.

The side effect here, generally speaking, is your strings are stripped of a trailing newline character.

---

The second deadlock occurs when you go to read the child process' response.

Firstly, this example

char rbuf[1];                    
while(read(pfd[N][0], &rbuf, 1) > 0)
{
    write(STDOUT_FILENO, &rbuf, 1);
}

is malformed. Either remove the & operators, OR change char rbuf[1]; to char rbuf;. Fixing this, and the newline problem from above, will result in the parent process reading data back from the child.

The problem then becomes that a while (read(...) > 0) loop will continuously block execution of the calling process, waiting for more data to be available.

This means another deadlock when the child process has already moved on to trying to read another pair of lines from the parent process.

A simple solution is to attempt a single, reasonably large read in the parent, and rely on the behaviour of fflush(stdout); in the child to flush the pipe to the parent.

Here is a functional -ish example, with minimal changes made. This program still has some problems, such as: the parent process generally has no idea of the status of the child processes, and relying signal propagation (^C) from the terminal to end the process tree gracefully, since loopcmp does not handle EOF (should really discuss this with whoever wrote loopcmp.c / mygets).

Additionally, mygeti is flawed as well, as an invalid input cannot be distinguished from a valid input of 0. It also does not handle EOF, or prevent signed integer overflow.

Some more robust abstraction (functions and structures) around creating child processes would help a lot to clean this up further.

This should help you progress, though.

#define _POSIX_C_SOURCE 200809L
#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/wait.h>
#include <unistd.h>

#define LINELEN (80)

char *mygets(char *buf, int len);
int mygeti();

void close_pipe(int fd[2])
{
    close(fd[0]);
    close(fd[1]);
}

int main(void)
{
    char *cmpstr[] = {"lexcmp", "lencmp"};
    int veclen = sizeof(cmpstr)/sizeof(char *);
    char str1[LINELEN + 1];
    char str2[LINELEN + 1];
    int index;
    int pid[2];
    int pfd[4][2];

    /* pfd[0] is TO lexcmp
     * pfd[1] is TO lencmp
     * pfd[2] is FROM lexcmp
     * pfd[3] is FROM lencmp
     */

    for(int i = 0; i < 4; i++)
        if(pipe(pfd[i]) < 0) {
            perror("pipe");
            return -2;
        }

    pid[0] = fork();

    if (pid[0] == 0) {
        /* child lexcmp */
        close_pipe(pfd[1]);
        close_pipe(pfd[3]);

        close(pfd[0][1]);
        close(pfd[2][0]);

        dup2(pfd[0][0], STDIN_FILENO);
        dup2(pfd[2][1], STDOUT_FILENO);
        close(pfd[0][0]);
        close(pfd[2][1]);

        char *args[] = { "./loopcmp", "lexcmp", NULL };
        execvp(*args, args);
        perror("exec");
        return -2; /* This only returns from the child */

    }

    pid[1] = fork();

    if (pid[1] == 0) {
        /* child lencmp */
        close_pipe(pfd[0]);
        close_pipe(pfd[2]);

        close(pfd[1][1]);
        close(pfd[3][0]);

        dup2(pfd[1][0], STDIN_FILENO);
        dup2(pfd[3][1], STDOUT_FILENO);

        close(pfd[1][0]);
        close(pfd[3][1]);

        char *args[] = { "./loopcmp", "lencmp", NULL };
        execvp(*args, args);
        perror("exec");
        return -2; /* This only returns from the child */
    }

    /* parent */

    close(pfd[0][0]);
    close(pfd[1][0]);
    close(pfd[2][1]);
    close(pfd[3][1]);

    while (1) {
        printf("Please enter first string: ");
        if (mygets(str1, LINELEN) == NULL)
            break;
        printf("Please enter second string: ");
        if (mygets(str2, LINELEN) == NULL)
            break;

        do {
            printf("Please choose (");
            for (int i=0 ; i < veclen ; i++)
                printf(" [%d] %s", i, cmpstr[i]);
            printf(" ): ");
            index = mygeti();
        } while ((index < 0) || (index >= veclen));

        if (0 >= dprintf(pfd[index][1], "%s\n%s\n", str1, str2)) {
            fprintf(stderr, "Failed to write to child %d\n", index);
            perror("dprintf");
            return -2;
        }

        char buf[64];
        ssize_t bytes = read(pfd[index + 2][0], buf, sizeof buf - 1);

        if (-1 == bytes) {
            perror("read from child");
            return -2;
        }

        buf[bytes] = 0;
        printf("Result: %s", buf);
    }
}

char *mygets(char *buf, int len)
{
    char *retval;

    retval = fgets(buf, len, stdin);
    buf[len] = '\0';
    if (buf[strlen(buf) - 1] == 10) /* trim \r */
        buf[strlen(buf) - 1] = '\0';
    else if (retval)
        while (getchar() != '\n'); /* get to eol */

    return retval;
}

int mygeti()
{
    int ch;
    int retval=0;

    while(isspace(ch=getchar()));
    while(isdigit(ch))
    {
        retval = retval * 10 + ch - '0';
        ch = getchar();
    }
    while (ch != '\n')
        ch = getchar();
    return retval;
}

Note the use of dprintf. If not available for whatever reason, just make sure to write a single newline after each string.

---

Final aside: with the way fgets works, the + 1 to the string buffer sizes are rather meaningless (although they are indirectly required here due to mygets performing its own, poorly designed buf[len] = '\0'). fgets writes at most len - 1 non-null bytes, always leaving room for the null terminating byte, which it places.