15

I have to write a shell that can run pipes. For example commands like ls -l | wc -l". I have successfully parsed the command given by the user as below:

"ls" = firstcmd

"-l" = frsarg

"wc" = scmd

"-l" = secarg

Now I have to use two forks since the commands are two and a pipe. The code block that I wrote to exec the command is the following:

pid_t pid;
int fd[2];

pipe(fd);
pid = fork();

if(pid==0)
{        
    dup2(fd[WRITE_END], STDOUT_FILENO);
    close(fd[READ_END]);
    execlp(firstcmd, firstcmd, frsarg, (char*) NULL);
}
else
{ 
    pid=fork();

    if(pid==0)
    {
        dup2(fd[READ_END], STDIN_FILENO);
        close(fd[WRITE_END]);
        execlp(scmd, scmd, secarg, (char*) NULL);
    }
}

So when I run my shell and I enter the command ls -l | wc -l (for example) the result from the execs doesn't show up but the shell keeps running normally.

The strange thing is that the result of the command shows only when I terminate my shell with "exit" or "^C".

What is wrong with that output? Why doesn't it show up right after I enter the command?

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Aris Kantas
  • 375
  • 1
  • 5
  • 15
  • 1
    You need to close the pipe FDs in the parent processes. – Barmar Nov 24 '15 at 02:19
  • Can you edit my code to help me understand what you mean please? @Barmar – Aris Kantas Nov 24 '15 at 02:22
  • 3
    **Rule of thumb**: if you duplicate one end of a pipe to standard input or standard output, you should close both ends of the original pipe before using `exec*()` functions (or otherwise continuing). There are exceptions; they are few and far between. It is very seldom (on SO, and IRL) that you encounter a program using pipes that closes too many descriptors; it is very common to find a program that doesn't close enough descriptors. It's especially common if there are multiple children and/or multiple pipes to confuse things. – Jonathan Leffler May 19 '17 at 15:07
  • 1
    Incidentally, if you look at the code in https://github.com/jleffler/soq/tree/master/src/so-4380-8114, you'll find an interesting example where 'not closing enough file descriptors caused the program to fail on big enough inputs'. When the input was small, it worked fine. When the input was big enough, the program jammed in deadlock because pipes were not closed properly. (No, the jamming code isn't directly on Stack Overflow.) – Jonathan Leffler May 19 '17 at 15:22

3 Answers3

27

You need to close all the pipe descriptors in both the parent process and the child process (after duplication in the child process). In your code the main issue is that, the wc process does not exit because there are still writers present (since the parent process has not closed the write end). Changes shown below. I have also added the waitpid in the parent process to wait for the wc process.

pid_t pid;
int fd[2];

pipe(fd);
pid = fork();

if(pid==0)
{
    dup2(fd[WRITE_END], STDOUT_FILENO);
    close(fd[READ_END]);
    close(fd[WRITE_END]);
    execlp(firstcmd, firstcmd, frsarg, (char*) NULL);
    fprintf(stderr, "Failed to execute '%s'\n", firstcmd);
    exit(1);
}
else
{ 
    pid=fork();

    if(pid==0)
    {
        dup2(fd[READ_END], STDIN_FILENO);
        close(fd[WRITE_END]);
        close(fd[READ_END]);
        execlp(scmd, scmd, secarg,(char*) NULL);
        fprintf(stderr, "Failed to execute '%s'\n", scmd);
        exit(1);
    }
    else
    {
        int status;
        close(fd[READ_END]);
        close(fd[WRITE_END]);
        waitpid(pid, &status, 0);
    }
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
user1969104
  • 2,340
  • 14
  • 15
  • Oh yes. That's the most correct answer i have seen. It works perfectly. Thank you so much. I was stuck with it for hours. @user1969104 – Aris Kantas Nov 24 '15 at 12:47
3

Hmm, close enough. You miss to handle close on some file descriptor after fork.

Here is some reference:

  1. About pipe, http://unixwiz.net/techtips/remap-pipe-fds.html
  2. About file descriptor, http://www.usna.edu/Users/cs/aviv/classes/ic221/s14/lec/09/lec.html

Here's my code:

#include  <fcntl.h>                              //
#include  <stdio.h>                              //
#include  <stdlib.h>                             //
#include  <string.h>                             //
#include  <sys/types.h>                          //
#include  <sys/wait.h>                           //
#include  <sys/stat.h>                           //
#include  <termios.h>                            //
#include  <unistd.h>                             //
                                                 //
#define INPUT_END 1                              // INPUT_END means where the pipe takes input
#define OUTPUT_END 0                             // OUTPUT_END means where the pipe produces output
                                                 //
int main(int argc, char* argv[])                 //
{                                                //
    pid_t pid1;                                  // [STDIN -> terminal_input, STDOUT -> terminal_output]                       (of the parent process)
    pid_t pid2;                                  //
    int fd[2];                                   //
                                                 //
    pipe(fd);                                    // [STDIN -> terminal_input, STDOUT -> terminal_output, fd[0] -> pipe_input, fd[1] -> pipe_output]
    pid1 = fork();                               //
                                                 //
    if(pid1==0)                                  //
    {                                            // I am going to be the wc process (i.e. taking input from the pipe)
        close(fd[INPUT_END]);                    // [STDIN -> terminal_input, STDOUT -> terminal_output, fd[1] -> pipe_output] (of the WC process)
        dup2(fd[OUTPUT_END], STDIN_FILENO);      // [STDIN -> pipe_output, STDOUT -> terminal_output, fd[1] -> pipe_output]    (of the WC process)
        close(fd[OUTPUT_END]);                   // [STDIN -> pipe_output, STDOUT -> terminal_output]                          (of the WC process)
        execlp("wc", "wc", "-l",(char*) NULL);   //
    }                                            //
    else                                         //
    {                                            //
        pid2=fork();                             //
                                                 //
        if(pid2==0)                              //
        {                                        // I am going to be the ls process (i.e. producing output to the pipe)
            close(fd[OUTPUT_END]);               // [STDIN -> terminal_input, STDOUT -> terminal_output, fd[0] -> pipe_input] (of the ls process)
            dup2(fd[INPUT_END], STDOUT_FILENO);  // [STDIN -> terminal_input, STDOUT -> pipe_input, fd[0] -> pipe_input]      (of the ls process)
            close(fd[INPUT_END]);                // [STDIN -> terminal_input, STDOUT -> pipe_input]                           (of the ls process)
            execlp("ls","ls","-l",(char*) NULL); //
        }                                        //
                                                 //
        close(fd[OUTPUT_END]);                   // [STDIN -> terminal_input, STDOUT -> terminal_output, fd[0] -> pipe_input] (of the parent process)
        close(fd[INPUT_END]);                    // [STDIN -> terminal_input, STDOUT -> terminal_output]                      (of the parent process)
        waitpid(-1, NULL, 0);                    // As the parent process - we wait for a process to die (-1) means I don't care which one - it could be either ls or wc
        waitpid(-1, NULL, 0);                    // As the parent process - we wait for the another process to die.
                                                 // At this point we can safely assume both process are completed
    }                                            //
}                                                //
Andrew Au
  • 812
  • 7
  • 18
ibrohimislam
  • 717
  • 7
  • 21
  • 1
    There are issues in your code as well. First INPUT_END/OUTPUT_END has incorrect values as per the documentation of `pipe`. To fix this issue, you have interchanged `wc` and `ls` commands. Finally, you have not closed the descriptors in the parent process. – user1969104 Nov 24 '15 at 03:35
  • @user1969104 I run your code and it worked but i found another problem. For your information my code (too big to write it here) uses other execs in case the user gives only **"ls"** or **"ls -l"** or **"ls -l /bin"**. In general commands with maximum two arguments. When the user gives the command **"ls -l | wc -l"** everything is ok the result is shown right away but when after that command i try only **"ls"** or any other command with no pipe nothing happens. Why is that happening? – Aris Kantas Nov 24 '15 at 18:44
  • I am not sure what you mean without seeing your code, but please make sure that the pipe redirection (calling pipe, fork, dup2) happens only when there is a second command given in the argument. Otherwise, if you just call `execlp("ls","ls","-l",(char*) NULL)`, there is no reason why there is no output. – user1969104 Nov 25 '15 at 05:49
  • 2
    Note that `using namespace std;` is a syntax error in C unless you've got an unusual collection of macros defined on your compiler command line (since those macros won't be defined in system headers). Please don't post C++ code as an answer to a C question. You also have multiple unused headers in the list; that's not very elegant. ``, ``, ``, ``, are unused, and modern POSIX doesn't require `` to be included explicitly. You should at least exit if `execlp()` returns; arguably, you should print an error message too. – Jonathan Leffler May 19 '17 at 15:12
-5

Well, a simple and efficient work arund for something like this is making a script with the pipe stuff and then calling the script with some exec command from your C code.

script.sh

#!/bin/sh
ls -l | wc -l

And the you just do in your C program something like this

char *argv[] = {"script.sh", NULL};
execv(argv[0], argv);

Note that you'll have to have the script.sh in the same directory of your C program.

Delphoenyx
  • 19
  • 2
  • 3
    Yeah, write a shell to execute that script that pipes and then have the shell call itself to execute a script that pipes. See how long it takes you to run out of resources. – S.S. Anne Mar 17 '20 at 21:39