CODEWITHSUNDEEP
cunixpipefork
breakthatbass
breakthatbass

Reputation: 376

Why does wc command in execvp() return the wrong results when taking input from a pipe in my shell?

I'm working on writing a shell in C for learning purposes and I'm trying to allow for a variable number of pipes. In general, it seems to work great. But I noticed a problem with the wc command.

When I pipe some output of another program into wc like ls | wc it always returns
1 3 35 no matter what I pipe into it. Other commands work as expected when I pipe into them. In my normal zsh shell wc works fine. I'm struggling to find the problem. I've tried adding waitpid after the forks but no dice.

Here's the main shell loop in the main function:

while (1) {
        printf("\033[31;1mshell:\033[0m ");

        line = read_cmds();

        if (strstr(line, "|")) {
            // check for pipes first
            pipe_exec(line);
        } else {
            // we have a single command
            tokens = split(line, " \t\r\n");
            if (*tokens != NULL) shell_exec(tokens);
            free(tokens);
        }
    }

Here is the function that loops through the commands:

void pipe_exec(char *line)
{
    int in, status;
    int pipe_no; // keep track of ptr to bring it back to free 
    int pfd[2];
    pid_t rc;
    char **cmd, **pipe_cmds;
    
    // split takes a string and splits into array of strings based on delimiter
    pipe_cmds = split(line, "|"); 

    in = 0;
    pipe_no = 0;
    while (*pipe_cmds) {
        cmd = split(*pipe_cmds, " \t\r\n");

        if (pipe(pfd) < 0) perror("pipe");

        make_proc(in, pfd[1], cmd);
        close(pfd[1]);
        in = pfd[0];
        pipe_cmds++; // move ptr ahead one
        pipe_no++;
    }
    // move pointer back and free
    pipe_cmds -= pipe_no;
    free(pipe_cmds);

    rc = fork();
    if (rc == 0) {
        if (in != 0) dup2(in, STDIN_FILENO);
        execvp(*cmd, cmd);
    } 
}

And then the make_proc function that the above function calls:

void make_proc(int in, int out, char **cmd)
{
    pid_t rc;
    rc = fork();
    if (rc  == 0) {
        if (in != STDIN_FILENO) {
            dup2(in, STDIN_FILENO);
            close(in);
        }
        if (out != STDOUT_FILENO) {
            dup2(out, STDOUT_FILENO);
            close(out);
        }
        execvp(*cmd, cmd);
    } 
}

I took out some of the error checking to save space here. Any help is appreciated!

Upvotes: 1

Views: 257

Answers (1)

Semion
Semion

Reputation: 136

You execute the last command twice and pipe its first instance to the second. Adding something like:

    while (*pipe_cmds) { 
        cmd = split(*pipe_cmds, " \t\r\n");
        if (!pipe_cmds[1]) {
            break;
        }

        if (pipe(pfd) < 0) perror("pipe");

        make_proc(in, pfd[1], cmd);
        close(pfd[1]);
        in = pfd[0];
        pipe_cmds++; // move ptr ahead one
        pipe_no++;
    }

would prevent the unnecessary instance, although I would rather have refactored this function a bit.

Upvotes: 1

Related Questions