C program with execvp cat doesn't take more than 150 lines input

Question

I'm writing a program that puts line numbers in front of every line of a given text document. I wrote this by using pipes since the point of this is learning how pipes work.

The problem: the program runs fine when there is a small amount of data in the input file (about 150 lines). When I put in more lines (200+), the output file is good until the 153rd line, then it repeats the last 40 lines or so.

When I put in way more lines (6000) the program never ends and the output file stays empty.

I have no clue where it goes wrong so it would be great to get some help with it.

Code :

#include <sys/types.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <wait.h>
#include <limits.h>

void Parent(int *ipPX, int *ipPY);
void Child(int *IpPX, int *IpPY);

FILE *ifp, *ofp; //input and output file
void fileHandling (char *argv[2]);

int main(int argc, char *argv[]) {
    pid_t pid;
    int pdX[2], pdY[2];

    fileHandling(argv);
    pipe(pdX);
    pipe(pdY); //make new file descriptors in table
    switch (pid = fork()) {
        case 0: Child(pdX, pdY);
            break;
        case -1: perror("Error creating child");
            exit(1);
        default: Parent(pdX, pdY);
    }
    exit(EXIT_SUCCESS);
}

void Parent(int *ipPX, int *ipPY) {
    char *line = NULL;
    size_t len = 0;
    ssize_t readedLines;
    char buf[LINE_MAX];
    int n;

    close(ipPX[0]);close(ipPY[1]); //close not used fd's

    while ((readedLines = getline(&line, &len, ifp)) > 0) {
        write(ipPX[1], line, readedLines); //read from input file and write to cat process
    }
    close(ipPX[1]); //done with this fd

    while ((n = read(ipPY[0], buf, sizeof (buf))) > 0) {
        fprintf(ofp, "%s", buf); //read from cat process and print to file
    }
    close(ipPY[0]); //done with this fd

    wait(0); //wait for child to finish
}

void Child(int *ipPX, int *ipPY) {
    close(ipPX[1]);
    close(ipPY[0]);
    close(0);
    dup(ipPX[0]);
    close(ipPX[0]);
    close(1);
    dup(ipPY[1]);
    close(ipPY[1]);
    execlp("cat", "cat", "-n", NULL);
    perror("Execlp error"); //not reached unless execlp fails
    exit(1);
}

void fileHandling (char *argv[2]){
    char *outputFile; //name of output file 

     if (!(ifp = fopen(argv[1], "r"))) {
        fprintf(stderr, "Error opening input file");
        exit(1);
    }
     if (argv[2] == NULL) {
         outputFile = "LineNumbersOutput";
     }
     else {
         outputFile = argv[2];
     }
    if (!(ofp = fopen(argv[2], "w+"))) {
        fprintf(stderr, "Error opening output file");
        exit(1);
    }
}

UPDATE: Now i added the 3rd process, but the line numbers that got added in previous code are gone. The output is complete now (no matter how big the file)

thanks for all the great help!

New code :

#include <sys/types.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <wait.h>
#include <limits.h>

void Parent(int *ipPX, int *ipPY);
void CatProcess(int *IpPX, int *IpPY);
void WriteToFile(int *ipPX, int *ipPY);

FILE *ifp, *ofp; //input and output file
void FileHandling (char *argv[2]);

int main(int argc, char *argv[]) {
    pid_t catpid;
    pid_t writeToFile;

    int pdX[2], pdY[2];

    FileHandling(argv);
    pipe(pdX);
    pipe(pdY); //make new file descriptors in table

    //first child 
    switch (catpid = fork()) {
        case 0: CatProcess(pdX, pdY);
            break;
        case -1: perror("Error creating Cat child");
            exit(1);
        default: 
        writeToFile = fork();
    }

    //second child
    switch (writeToFile){
        case 0: WriteToFile(pdX, pdY);
            break;
        case -1: perror("Error creating writeToFile child");
            exit(1);
        default: 
            Parent(pdX, pdY);
    }

    exit(EXIT_SUCCESS);
}

void Parent(int *ipPX, int *ipPY) {
    char *line = NULL;
    size_t len = 0;
    ssize_t readedLines;

    close(ipPX[0]);close(ipPY[1]); close(ipPY[0]);//close not used fd's

    while ((readedLines = getline(&line, &len, ifp)) > 0) {
        write(ipPX[1], line, readedLines); //read from input file and write to cat process
    }
    close(ipPX[1]); //done with this fd

    wait(0); //wait for child to finish
}

void CatProcess(int *ipPX, int *ipPY) {
    close(ipPX[1]);
    close(ipPY[0]);
    close(0);
    dup(ipPX[0]);
    close(ipPX[0]);
    close(1);
    dup(ipPY[1]);
    close(ipPY[1]);
    execlp("cat", "cat", "-n", NULL);
    perror("Execlp error"); //not reached unless execlp fails
    exit(1);
}

void FileHandling (char *argv[2]){
    char *outputFile; //name of output file 

     if (!(ifp = fopen(argv[1], "r"))) {
        fprintf(stderr, "Error opening input file");
        exit(1);
    }
     if (argv[2] == NULL) {
         outputFile = "LineNumbersOutput";
     }
     else {
         outputFile = argv[2];
     }
    if (!(ofp = fopen(argv[2], "w+"))) {
        fprintf(stderr, "Error opening output file");
        exit(1);
    }
}

void WriteToFile(int *ipPY,int *ipPX){
   char buf[LINE_MAX];
   int n;
   close(ipPY[1]); close(ipPX[0]); close(ipPX[1]); //close unused fd's
   close(0); dup(ipPY[0]); close(ipPY[0]); //redirect stdin
   while ((n = read(0, buf, sizeof (buf))) > 0) {
        fprintf(ofp, "%s", buf); //read from cat process and print to file
    }
 }

made a drawing to make it clear for myself (think it is correct)

Answer 1

Updated answer

The new code is almost working — but there's a tiny (but critical) inconsistency:

void Parent(int *ipPX, int *ipPY) { ... }
void CatProcess(int *ipPX, int *ipPY) { ... }
void WriteToFile(int *ipPY,int *ipPX){ ... }

The code in WriteToFile() is written on the assumption that the arguments are in the order ipPX, ipPY. When you change the definition, the code works reasonably well. I eventually debugged the problem by printing out the file descriptor assignments in the three functions and spotting (it wasn't hard!) that the descriptors were reversed in WriteToFile().

You should also fix the fprintf().

Note that there was zero chance of debugging this without the code to inspect. Here's the working code as (mildly) modified by me.

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

void Parent(int *ipPX, int *ipPY);
void CatProcess(int *IpPX, int *IpPY);
void WriteToFile(int *ipPX, int *ipPY);

FILE *ifp, *ofp; //input and output file
void FileHandling(int argc, char *argv[2]);

int main(int argc, char *argv[]) {
    pid_t catpid;
    pid_t writeToFile;
    int pdX[2], pdY[2];

    FileHandling(argc, argv);
    pipe(pdX);
    pipe(pdY); //make new file descriptors in table

    //first child 
    switch (catpid = fork()) {
        case 0:
            CatProcess(pdX, pdY);
            break;
        case -1:
            perror("Error creating Cat child");
            exit(1);
        default: 
            writeToFile = fork();
            break;
    }

    //second child
    switch (writeToFile){
        case 0:
            WriteToFile(pdX, pdY);
            break;
        case -1:
            perror("Error creating writeToFile child");
            exit(1);
        default: 
            Parent(pdX, pdY);
            break;
    }

    exit(EXIT_SUCCESS);
}

void Parent(int *ipPX, int *ipPY)
{
    char *line = NULL;
    size_t len = 0;
    ssize_t readedLines;
    fprintf(stderr, "\n%d: mom: %d -> %d, %d -> %d\n",
            (int)getpid(), ipPX[1], ipPX[0], ipPY[1], ipPY[0]);

    close(ipPX[0]);
    close(ipPY[1]);
    close(ipPY[0]);

    while ((readedLines = getline(&line, &len, ifp)) > 0)
        write(ipPX[1], line, readedLines); //read from input file and write to cat process
    close(ipPX[1]); //done with this fd
    fclose(ifp);
    ifp = 0;
    free(line);

    wait(0); //wait for child to finish
}

void CatProcess(int *ipPX, int *ipPY)
{
    fprintf(stderr, "\n%d: cat: %d -> %d, %d -> %d\n",
            (int)getpid(), ipPX[1], ipPX[0], ipPY[1], ipPY[0]);
    close(ipPX[1]);
    close(ipPY[0]);
    close(0);
    dup(ipPX[0]);
    close(ipPX[0]);
    close(1);
    dup(ipPY[1]);
    close(ipPY[1]);
    execlp("cat", "cat", "-n", NULL);
    perror("execlp error"); //not reached unless execlp fails
    exit(1);
}

void FileHandling(int argc, char *argv[2])
{
    char *outputFile; //name of output file 

    if (argc < 2 || argc > 3)
    {
        fprintf(stderr, "Usage: %s input [output]\n", argv[0]);
        exit(1);
    }

    if (!(ifp = fopen(argv[1], "r"))) {
        fprintf(stderr, "Error opening input file");
        exit(1);
    }
    if (argv[2] == NULL)
        outputFile = "LineNumbersOutput";
    else
        outputFile = argv[2];
    if (!(ofp = fopen(outputFile, "w+"))) {
        fprintf(stderr, "Error opening output file");
        exit(1);
    }
}

void WriteToFile(int *ipPX,int *ipPY)
{
    char buf[LINE_MAX];
    int n;
    fprintf(stderr, "\n%d: dog: %d -> %d, %d -> %d\n",
            (int)getpid(), ipPX[1], ipPX[0], ipPY[1], ipPY[0]);
    close(ipPY[1]);
    close(ipPX[0]);
    close(ipPX[1]);

    close(0);
    dup(ipPY[0]);
    close(ipPY[0]); //redirect stdin
    while ((n = read(0, buf, sizeof(buf))) > 0)
        fprintf(ofp, "%.*s", n, buf);
}

The <wait.h> header is non-standard; the standard header is <sys/wait.h>.

Original answer

Your problem is that pipes have a finite capacity, which can be as small as 4 KiB according to POSIX, was traditionally 5 KiB, is 64 KiB on Mac OS X, and is 64 KiB on an ancient Linux (SuSE 10). There comes a point, therefore, where with the best will in the world, you can't send any more data to cat because you've not read anything back from it and both the pipes are full.

How to resolve?

You could consider a three-process solution:

1. Process P1 reads file and writes to P2.
2. P2 is cat -n, and its output goes to P3.
3. P3 reads its standard input and writes it to standard output.

All this is remembering that the point of the exercise is to use pipes — if you just wanted to get the job done, you'd simply execute cat -n with its input coming from file 1 and output going to file 2.

Alternatively, you can use non-blocking reads. Add the O_NONBLOCK setting to the input file descriptor of the pipe back from cat -n (use fcntl() — F_GETFL and F_SETFL). In your main loop, every time you write a line, you would attempt to read a line back from child. If you get something, write it out; if not, never mind. When you've finished writing to the child, close the output pipe (to let cat know there will be no more data) and set the read pipe back to blocking and finish reading from the child; when the read returns 0 bytes this time, you're done.

pipesize.c

#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int main(void)
{
    int fd[2];  /* Output pipe - for parent */
    char buffer[16] = "ABCDEFGHIJKLMNOP";

    if (pipe(fd) < 0)
    {
        fprintf(stderr, "Failed to create pipe\n");
        exit(1);
    }

    int p_flags = fcntl(fd[1], F_GETFL);
    p_flags |= O_NONBLOCK;
    fcntl(fd[1], F_SETFL, p_flags);
    size_t nbytes = 0;
    while (write(fd[1], buffer, sizeof(buffer)) == (ssize_t)sizeof(buffer))
        nbytes += sizeof(buffer);
    printf("PIPE buffer size: %zu bytes\n", nbytes);

    return 0;
}

This program determines the capacity of a pipe. A similar program to determine the capacity of a FIFO shows that the capacity of a FIFO is 8 KiB on Mac OS X and 64 KiB again on Linux.

There are also configuration parameters that you can look up for these — see pathconf() — but where's the fun in that.