pipe function returning random characters

Question

    #include <stdio.h>
    #include <unistd.h>
    #include <stdlib.h>
    #include <string.h>
    #include <sys/types.h>
    
    int main(void){
    
    //Variables, p[2] for each end of the pipe. nbytes to read pipe return value SUCCESS or FAILURE. pid_t to hold pid of fork process.
    // buffer to hold response from the child process.
        int     p[2], nbytes;
        pid_t   childpid;
        char    string[] = "Hello, World!\n";
        char    buffer[80];
    
    //Declaration of pipe
        pipe(p);
        
        //Error handling.
        if(((childpid = fork()) == -1) || (pipe(p) == -1))
        {
                perror("fork");
                exit(1);
        }
    
        //Child process sends message to paprent.
        if(childpid == 0)
        {
                /* Child process closes up input side of pipe */
                close(p[0]);
    
                /* Send "string" through the output side of pipe */
                write(p[1], string, (strlen(string)+1));
                exit(0);
        }
        else
        {
                /* Parent process closes up output side of pipe */
                close(p[1]);
    
                /* Read in a string from the pipe */
                nbytes = read(p[0], buffer, sizeof(buffer));
    
                printf("Received string: %s", buffer);
    
        }
        return(0);
    }

Output > Received string: @�=zJ

The point of the exercise is to have a child process send a message through a pipe to the parent process and the parent returns the result. This exact code worked the first time I ran it, but then when I tried to run it a second time it started to return seemingly random characters each time. I tried to copy my buffer to another variable but then it was empty. Is the pipe actually not function the way I think it is? What am I doing wrong?

Answer 1

You have:

pipe(p);

//Error handling.
if(((childpid = fork()) == -1) || (pipe(p) == -1))
{
        perror("fork");
        exit(1);
}

This creates two pipes — one in the line pipe(p); and the second in the condition if(((childpid = fork()) == -1) || (pipe(p) == -1)). This is wasteful at best. Moreover, the second pipe is after the fork(), so the parent and child processes don't access the same pipe any more — you overwrote the one created before the fork() which they do share. Test the result of pipe() before calling fork() and remove the extra condition in the if test:

if (pipe(p) != 0)
{
    perror("pipe");
    exit(1);
}
if ((childpid = fork()) < 0)
{
    perror("fork");
    exit(1);
}

Get used to testing for errors and writing appropriate code to handle them. It will be a major part of your life as a C programmer.

Later on in the code, you have:

{
        /* Parent process closes up output side of pipe */
        close(p[1]);

        /* Read in a string from the pipe */
        nbytes = read(p[0], buffer, sizeof(buffer));

        printf("Received string: %s", buffer);

}

You need to heed the value of nbytes. Since it is an int, you could use:

printf("Received %d bytes: [%.*s]\n", nbytes, nbytes, buffer);

This limits the output to what was read, and reports 0 if that's what it gets. I suppose you should also check for -1 in nbytes before using it in the printf() statement:

if (nbytes < 0)
{
    fprintf(stderr, "failed to read from pipe descriptor %d\n", p[0]);
    // Or perror("read");
    // Should you exit here with a non-zero status?
}
else
    printf("Received %d bytes: [%.*s]\n", nbytes, nbytes, buffer);

Note: errors are reported on stderr; perror() does that automatically.

Answer 2

You first create a pipe with pipe(p); and then you create another with ... || (pipe(p) == -1)) Is that deliberate?

2nd Pipe was causing an issue.

Answer 3

The problem is that you create two pipes when you really only need to check the first for errors:

// Declaration of pipe
if(pipe(p) == -1) {               // check for error here
    perror("pipe");
    exit(1);
}

// Error handling.
if((childpid = fork()) == -1) { // and don't create another pipe here
    perror("fork");
    exit(1);
}

You should also check the return values from write and read. They may not write or read the full string in one go.