Implement multiple pipeline with builtin

I’m stuck on Stage #NY9

I’ve tried to implement this features using multiprocessing and pipe.when i run my executable (./shell) it gives right output but tester fails.

Here are my logs:

[tester::#NY9] ^ Line does not match expected value.
[tester::#NY9] Expected: "       1       1      16"
[tester::#NY9] Received: "$ " (trailing space)

And here’s a snippet of my code:

 int fd[2];

        int prev_pipe_read_end = 0;
        std::vector<pid_t> child_pids;

        int original_stdin = dup(STDIN_FILENO);
        int original_stdout = dup(STDOUT_FILENO);
        // Flag to determine if we need to fork built-ins in pipeline
        // Since we have multiple commands, we should fork ALL commands
        //  bool is_pipeline = (cmd_queue.size() > 0);

        /*

             TODO_1: let do this with  first example : pwd | grep "pwd"
             1. parent writes on the pipe write end and store the read end for the next commadn
             2. First step output of pwd is written in write end of the pipe and store  its read  end for the next command
             3. Then for a grep which is not a builtin command.
             4. It reads fron the read end of the previoss pipe, execute accroding them -> ouput is written current pipe write end and store the read end in parent processs for next commaand

        */

        while (!cmd_queue.empty())
        {
            auto current_cmd = cmd_queue.front();
            cmd_queue.pop();
            bool is_last = cmd_queue.empty();

            if (!is_last)
            {
                if (pipe(fd) == -1)
                {
                    perror("pipe");
                    exit(1);
                }
            }

           
              // For pipelines, we fork for ALL commands (including built-ins)
            // This is simpler and more reliable
            pid_t pid = fork();
        
            if(pid == 0) {
                // Setup input from previous pipe
                if (prev_pipe_read_end != 0)
                {
                    dup2(prev_pipe_read_end, STDIN_FILENO);
                    close(prev_pipe_read_end);
                }

              
                 // Setup output to next pipe (if not last)
                if (!is_last)
                {
                    close(fd[0]);  // Close read end in child
                    dup2(fd[1], STDOUT_FILENO);
                    close(fd[1]);  // Close write end after dup2
                }

                // Handle redirections for the pipeline
                // Only apply output redirection to the last command
                if (is_last && ps.has_output_redirect())
                {
                    int out_fd = open(ps.get_output_file().c_str(),
                                     O_WRONLY | O_CREAT | (ps.is_append_mode() ? O_APPEND : O_TRUNC),
                                     0777);
                    if (out_fd != -1)
                    {
                        dup2(out_fd, STDOUT_FILENO);
                        close(out_fd);
                    }
                }

                // Apply error redirection to all commands in pipeline
                if (ps.has_error_redirect())
                {
                    int err_fd = open(ps.get_error_file().c_str(),
                                     O_WRONLY | O_CREAT | (ps.is_append_mode() ? O_APPEND : O_TRUNC),
                                     0777);
                    if (err_fd != -1)
                    {
                        dup2(err_fd, STDERR_FILENO);
                        close(err_fd);
                    }
                }


                 if (current_cmd.is_builtin)
                {
                    auto builtin_cmd = Builtin<Parser>::getMap()[current_cmd.cmd];
                    builtin_cmd->execute(current_cmd.argv);
                    fflush(stdout);
                    fflush(stderr);
                    exit(0);  // Important: Exit child after built-in execution
                }
                else
                {
                    execvp(current_cmd.cmd.c_str(), exec_vector(current_cmd.argv).data());
                    perror("execvp");
                    exit(1);
                }

            } else if(pid > 0) {
                child_pids.push_back(pid);
                
                // Close previous pipe read end in parent
                if (prev_pipe_read_end != 0)
                {
                    close(prev_pipe_read_end);
                }


                  // Setup for next command
                if (!is_last)
                {
                    close(fd[1]);  // Close write end in parent
                    prev_pipe_read_end = fd[0];  // Save read end for next command
                }
                else
                {
                    // Last command - close any remaining pipe read end
                    if (prev_pipe_read_end != 0)
                    {
                        close(prev_pipe_read_end);
                        prev_pipe_read_end = 0;
                    }
                }
            } else {
                perror("fork");
                exit(1);
            }
        

        
        }

        
        

        for (pid_t pid : child_pids)
        {
            int status;
            waitpid(pid, &status, 0);
        }


        // Restore original stdin/stdout
        dup2(original_stdin, STDIN_FILENO);
        dup2(original_stdout, STDOUT_FILENO);
        close(original_stdin);
        close(original_stdout);

Bit stumped too. The fact that you get no output of the form X Y Z suggest something is going on with wc or at least where its output ends up. Running wc with immediate EOF on stdin still outputs 0 0 0, but you get none.

I am assuming your echo implementation writes using printf, or fprintf to stdout?

Here is a random cause hypothetical. I am wondering whether there is some bad interaction (not standard allowed) between setting STDOUT_FILENO and stdout being out of sync? Normally that would not be a problem for execvp since it “refreshes” with a fresh c-runtime, but you run the built-ins forked from parent environment, so maybe there the STDOUT FILE does not have proper state (e.g. is buffer and position is partially full)?

I did also notice some mostly unrelated things.

  • You dup the original file descriptors in the parent and restore them. Doesn’t hurt, but also doesn’t look like the main parent process ever changes them.
  • I couldn’t see where ps is defined, but it looks like it exists “outer” from your command invocations (not part of current_cmd). Each cmd can have its own redirect I believe: cmdA foo 1>data.json | cmdB bar 2>errors.log.
  • In your parent you close its version of last read end, (prev_pipe_read_end) without setting it to 0, always if possible. Bit later (if is_last), you do it again. Fortunately close-after-close is safe.

My echo implementation is using cout . Is there a problem? If yes , why i got some wc in my shell

maybe it is bug

Can anyone tell how to pass this stage?

Completed the challenge. :saluting_face:

2 Likes

Can you describe the solution? Seems like a gnarly issue you encountered and may help others to know how you solved it.

1 Like

Sure. I made a silly mistake. so,I defined a member variable named bool is_valid_cmd inside the Parser class to distinguish between valid and invalid shell commands. However, I didn’t initialize it in the constructor. As a result, when I entered an invalid command, it was set to false. Then, when I entered a valid command, it still remained false, so the command was not executed and the shell waited for the next command instead.

Tester first test with non-valid command.

1 Like