0

I'm trying to hand pipes with the function below but it results in an infinite loop. My code is based off of this: Implementation of multiple pipes in C.

I'm new to C so I'd appreciate any thoughts or feedback on this solution. Please let me know if any other information would be required. There's no longer an infinite loop if I remove the third if statement, so the issue is somewhere there.

void command_with_pipes(char*** command_table, int count_pipes, bool converted_to_tee) {
    int fd[2 * count_pipes];
    pid_t pid; 
    int status; 

    // Create all necessary pipes at the beginning 
    for (int i = 0; i < count_pipes; i++) {
        if (pipe(fd + i*2) < 0) {
            perror("fatal error: pipe");
            exit(1);
        }
    }

    // Iterate over every command in the table 
    for (int commandNum = 0; commandNum <= count_pipes; commandNum++){
        pid = fork(); 
        if (pid < 0){
            perror("fork error"); 
            exit(-1);
        }

        else if (pid == 0){
            // If it's not the first command, should read to the pipe 
            if (commandNum != 0) {
                if(dup2(fd[(commandNum - 1) * 2], 0) < 0) {
                    perror("2 can't dup");
                    exit(1);
                }
            }

            // if not the last command, should write to the pipe 
            if (commandNum != count_pipes) {
                if(dup2(fd[commandNum  * 2 + 1], 1) < 0) {
                    perror("1 can't dup");
                    exit(1);
                }
            }

            // Last command includes a converted tee, should write to pipe instead of going to standard output 
            if (commandNum == count_pipes && strcmp(command_table[commandNum][0], "tee") ==0 && converted_to_tee == true){
                if(dup2(fd[(commandNum - 1) * 2 + 1], 1) < 0) {
                    perror("1 can't dup");
                    exit(1);
                }
            }

            // Close pipes 
            for (int i = 0; i < 2 * count_pipes; i++){
                close(fd[i]);
            }

            // Execute the command 
            status = execvp(command_table[commandNum][0], command_table[commandNum]);
            if (status < 0) {
                perror("exec problem");
                exit(1);
            }
        }
    }
    // Parent closes everything at the end 
    for (int i = 0; i < 2 * count_pipes; i++){
        close(fd[i]);
    }
    wait(0);
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Amy Wang
  • 113
  • 2
  • 9
  • 2
    Should probably use `strcmp` here: `command_table[commandNum][0] == "tee"`. – Fiddling Bits Oct 15 '18 at 21:09
  • I'm using char*** because it's a 2D array of strings – Amy Wang Oct 15 '18 at 21:10
  • @FiddlingBits Good catch - I changed that but it didn't fix the issue. – Amy Wang Oct 15 '18 at 21:22
  • 1
    I'd like to see an MCVE ([MCVE]). It would show the data structure that you call this code with. You should probably avoid the input parsing code and simply initialize the data structures. What command line are you simulating? Maybe you should remove the specialization for `tee` (why do you think it is a good idea to treat it differently from any other command? — IMO, it should be treated the same as any other command) until everything else is working OK. – Jonathan Leffler Oct 15 '18 at 22:46
  • 1
    In general, if you fork N processes in the pipeline, you're going to need N calls to `wait()` to collect the corpses of the dead child processes. You have one call to `wait()`, AFAICS, which isn't enough. You should add more diagnostic printing, with the PID of the process doing the printing. You should report which children you create, and which one you collect the corpses for, and the exit statuses of those children. This is debug-only code, but invaluable. Make sure you print to `stderr` or `fflush(stdout)` routinely — use functions for the job. – Jonathan Leffler Oct 15 '18 at 22:49
  • See also [C `#define` macro for debug printing](https://stackoverflow.com/questions/1644868/c-define-macro-for-debug-printing/1644898#1644898). – Jonathan Leffler Oct 15 '18 at 22:50

0 Answers0