4

Could anyone tell me what's wrong with this code?

In summary, it creates input and output pipes and fork-exec's the sort program. The parent reads the dictionary /usr/share/dict/words and writes it to the pipe that is dup2()'d to sort's standard in and, likewise, reads the output from it, printing it to the terminal (the standard output of the parent). Or, at least, that's what's supposed to be happening.

A backtrace says that the parent hangs at the read() on line 130 (marked with the comment 'XXX'). It's almost as though sort isn't aware of the end-of-file, but closing the write end of pipeIn should 'signal' this, right?

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>

int main(int argc, char** argv)
{
    int pipeIn[2];
    int pipeOut[2];

    if ((pipe(pipeIn)) == -1)
    {
        perror("pipe");
        exit(EXIT_FAILURE);
    }

    if ((pipe(pipeOut)) == -1)
    {
        perror("pipe");
        exit(EXIT_FAILURE);
    }

    pid_t child = fork();

    if (child == 0)
    {
        // This is child!

        if ((dup2(pipeIn[0], STDIN_FILENO)) == -1)
        {
            perror("dup2");
            exit(EXIT_FAILURE);
        }

        if ((dup2(pipeOut[1], STDOUT_FILENO)) == -1)
        {
            perror("dup2");
            exit(EXIT_FAILURE);
        }

        if ((dup2(pipeOut[1], STDERR_FILENO)) == -1)
        {
            perror("dup2");
            exit(EXIT_FAILURE);
        }

        if ((close(pipeIn[0])) == -1)
        {
            perror("close");
            exit(EXIT_FAILURE);
        }

        if ((close(pipeOut[1])) == -1)
        {
            perror("close");
            exit(EXIT_FAILURE);
        }

        if ((execlp("sort", "-r", NULL)) == -1)
        {
            perror("execlp");
            exit(EXIT_FAILURE);
        }
    }
    else if (child == -1)
    {
        perror("fork");
        exit(EXIT_FAILURE);
    }
    else
    {
        // This is parent!

        if ((close(pipeIn[0])) == -1)
        {
            perror("close");
            exit(EXIT_FAILURE);
        }

        if ((close(pipeOut[1])) == -1)
        {
            perror("close");
            exit(EXIT_FAILURE);
        }

        int dict = open("/usr/share/dict/words", O_RDONLY);

        if (dict == -1)
        {
            perror("open");
            exit(EXIT_FAILURE);
        }

        char buf[1024];
        int count;

        while ((count = read(dict, buf, sizeof(char) * 1024)) > 0)
        {
            putchar('.');

            if ((write(pipeIn[1], buf, count)) == -1)
            {
                perror("write 1");
                exit(EXIT_FAILURE);
            }
        }

        if (count == -1)
        {
            perror("read");
            exit(EXIT_FAILURE);
        }

        if ((close(dict)) == -1)
        {
            perror("close");
            exit(EXIT_FAILURE);
        }

        if ((close(pipeIn[1])) == -1)
        {
            perror("close");
            exit(EXIT_FAILURE);
        }

        while ((count = read(pipeOut[0], buf, sizeof(char) * 1024)) > 0) // XXX
        {
            putchar('!');

            if ((write(STDOUT_FILENO, buf, count)) == -1)
            {
                perror("write 2");
                exit(EXIT_FAILURE);
            }
        }

        if (count == -1)
        {
            perror("read");
            exit(EXIT_FAILURE);
        }

        if ((close(pipeOut[0])) == -1)
        {
            perror("close");
            exit(EXIT_FAILURE);
        }
    }

    return EXIT_SUCCESS;
}

Thank you for any input (pardon the pun).

Doddy
  • 1,311
  • 1
  • 17
  • 31
  • possible duplicate of [Having trouble with fork(), pipe(), dup2() and exec() in C](http://stackoverflow.com/questions/916900/having-trouble-with-fork-pipe-dup2-and-exec-in-c) – wallyk Dec 19 '11 at 00:14
  • I've already read that and it seems rather different. – Doddy Dec 19 '11 at 00:16
  • I agree - the cross-referenced question is somewhat different. – Jonathan Leffler Dec 19 '11 at 03:37
  • You normally would not send `sort`'s standard error down the pipe; you'd leave it going to the same place as the standard error of the invoking process (which is likely the terminal). – Jonathan Leffler Dec 19 '11 at 03:39

1 Answers1

3

Your problem is that you are not closing the unused ends of your pipe in the chile process. So you need to add the following code somewhere before the exec

    if ((close(pipeIn[1])) == -1)
    {
        perror("close");
        exit(EXIT_FAILURE);
    }

    if ((close(pipeOut[0])) == -1)
    {
        perror("close");
        exit(EXIT_FAILURE);
    }
Sodved
  • 8,428
  • 2
  • 31
  • 43
  • +1 - You're right. It is counter-intuitive, but if you are using a pipe as standard input or standard output in a child process, then you normally end up closing **both** of the file descriptors returned by `pipe()`. – Jonathan Leffler Dec 19 '11 at 03:36
  • Yeah, I didn't get it at first, but kind of makes sense now that I've thought about it (which I never have until today). After the fork there are effectively two open file descriptiors which can write to the pipe. So you cannot get an `eof` on reading from the pipe until ALL the file descriptors for the writing end have been closed. – Sodved Dec 19 '11 at 05:49
  • Thanks, it's working, but it's not printing the list of words in reverse order. Is the syntax of `execlp("sort", "-r", NULL);` wrong? – Doddy Dec 19 '11 at 14:22
  • Oh, it needs to be `execlp("sort", "sort", "-r", NULL);` - I don't know why though! – Doddy Dec 19 '11 at 14:31
  • 1
    @bean Because `argv[0]` is always the name of the process. Lets you control how it shows up in `ps` and other process information. – Sodved Dec 19 '11 at 23:09