Thread: fork + pipe + dup2

  1. #1
    Registered User
    Join Date
    Sep 2010
    Posts
    4

    fork + pipe + dup2

    Hi,

    I'm developing a simple shell in C but I'm experiencing some problems to execute pipe separated commands properly, such as: 'env | sort' and others which make usage of the pipe '|'.

    As you may know, the pipe is the redirection of the stdout of the first command to the stdin of the second command. Therefore, in my program, when the user inputs commands in this format, my parser stores each command in a double linked list.

    If when this insertion is performed the list is empty, then there is no need to make a call to pipe since we must have at least one element inserted as explained before. But if that is the case, this is, if I have at least an element already inserted, then I make a call to (suppose e.g. pipe(fd)); and then I store fd[0] on the current element (which is the second command typed) and fd[1] on the previous element of the list (first command typed). This means that the current element (second) will be reading the output of the previous element, which will be writing to fd[1].

    Until now, no redirection is done but I've just set things to do that redirection properly.

    Then, after this linked list is completed, I perform some checks to validate it so I can run all commands perfectly.

    The process of running the command list is quite simple:
    1. Perform a fork;
    2. In the child code (pid == 0), I redirect its output to either fd[0] or fd[1] using the syscall dup2 which will be either dup2(fd[0], STDIN_FILENO) or dup2(fd[1], STDOUT_FILENO). Right after that I call close(fd[0]) or close(fd[1]) depending on what needs to be done by the child;
    3. Then I call execv with the command arguments to execute the typed command;


    The problem I'm having is that I cannot see the output of whole chain of commands on
    the shell. The last command typed does not perform any redirection of the stdout since it just receives different input since its stdin is redirected to its previous command. Everything is supposed to run correct.

    What am I doing wrong?

    Thanks in advance,
    Caio

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    You expect us to guess without seeing your code?
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  3. #3
    Registered User
    Join Date
    Sep 2010
    Posts
    4
    Quote Originally Posted by Salem View Post
    You expect us to guess without seeing your code?
    Without the code you could, at least, tell if the described logic is correct or not.

    Anyway, here is the logic for the process generation:

    Code:
    int genProcess(commandNode* node)
    {
        reservedNode* reserved = node->reservedL;
        int i = 0;
        int reservedCode = 0;
        char* fullPath = getFullPath(node->path, node->value);
        char** argv = NULL;
        pid_t pid;
        
        pid = fork();
    
        if( pid == 0 )
        {               
            /* Set process to read */
            if( node->fd[0] != VAR_UNINITIALIZED )
            {
                dup2(node->fd[0], STDIN_FILENO);
                close(node->fd[0]);
            }
            
            /* Set process to write */
            if( node->fd[1] != VAR_UNINITIALIZED )
            {
                dup2(node->fd[1], STDOUT_FILENO);
                close(node->fd[1]);
            }
        
            argv = (char**)malloc(sizeof(char*) * node->nArguments + 2);
            
            /* Set the executable (command) name */
            argv[i] = (char*) malloc (sizeof(char) * strlen(fullPath));
            memcpy(argv[i++], fullPath, sizeof(char) * strlen(fullPath));
            
            argumentNode* arg = node->argument;
            while( arg )
            {
                argv[i] = arg->value;
                arg = arg->next;
                i++;
            }
            argv[i] = NULL;
            
            /* Free the path returned before we change the forked image */
            free(fullPath);
            fullPath = NULL;
            
            /* Create an image of the executable and run it */
            execv(argv[0], argv);
        }
        else if(pid > 0)
        {
            printf("On parent\n");
        }
        else if(pid < 0)
        {
            printf("Fork has failed\n");
            
            free(fullPath);
            fullPath = NULL;
        }
    
        return pid;
    }

  4. #4
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    > Without the code you could, at least, tell if the described logic is correct or not.
    It's rather immaterial, when there are bugs elsewhere in your code.

    > argv[i] = (char*) malloc (sizeof(char) * strlen(fullPath));
    > memcpy(argv[i++], fullPath, sizeof(char) * strlen(fullPath));
    This neither allocates space for the \0 at the end of the string, nor does it COPY the \0 at the end of the string.

    If you get lucky, the very next byte in unallocated memory is \0
    If not, you either get "command not found from the exec call (who's result you do NOT check by the way).
    Or if you're really lucky, it runs off way into the distance looking for a \0 before finally segfaulting.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  5. #5
    Registered User
    Join Date
    Sep 2010
    Posts
    4
    Quote Originally Posted by Salem View Post
    > Without the code you could, at least, tell if the described logic is correct or not.
    It's rather immaterial, when there are bugs elsewhere in your code.

    > argv[i] = (char*) malloc (sizeof(char) * strlen(fullPath));
    > memcpy(argv[i++], fullPath, sizeof(char) * strlen(fullPath));
    This neither allocates space for the \0 at the end of the string, nor does it COPY the \0 at the end of the string.

    If you get lucky, the very next byte in unallocated memory is \0
    If not, you either get "command not found from the exec call (who's result you do NOT check by the way).
    Or if you're really lucky, it runs off way into the distance looking for a \0 before finally segfaulting.
    Yeah.
    I replaced the mentioned code by:

    argv[i] = (char*) malloc (strlen(fullPath) + 1);
    sprintf(argv[i++], "%s", fullPath);

    But this doesn't fix the problem of getting output to STDOUT.

    Regards,
    Caio

  6. #6
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    Here's a hacked up test environment, and some results.
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <sys/types.h>
    #include <unistd.h>
    #include <sys/wait.h>
    
    #define VAR_UNINITIALIZED   -1
    
    typedef struct argumentNode {
      char     *value;
      struct argumentNode *next;
    } argumentNode;
    
    typedef struct commandNode {
      int           fd[2];
      char          *path;
      int           nArguments;
      int           value;
      argumentNode  *argument;
    } commandNode;
    
    char *getFullPath(char *path, int value) {
      return path;
    }
    
    int genProcess(commandNode* node)
    {
        //reservedNode* reserved = node->reservedL;
        int i = 0;
        int reservedCode = 0;
        char* fullPath = getFullPath(node->path, node->value);
        char** argv = NULL;
        pid_t pid;
    
        pid = fork();
    
        if( pid == 0 )
        {
            /* Set process to read */
            fprintf(stderr,"cmd=%s\n",fullPath);
            if( node->fd[0] == VAR_UNINITIALIZED ) {
              fprintf(stderr,"C:Stdin Closed\n");
              close(STDIN_FILENO);
            }
            if( node->fd[0] > STDERR_FILENO )
            {
                fprintf(stderr,"C:Stdin redirected\n");
                dup2(node->fd[0], STDIN_FILENO);
                close(node->fd[0]);
            }
    
            /* Set process to write */
            if( node->fd[1] == VAR_UNINITIALIZED ) {
              fprintf(stderr,"C:Stdout closed\n");
              close(STDOUT_FILENO);
            }
            if( node->fd[1] > STDERR_FILENO )
            {
                fprintf(stderr,"C:Stdout redirected\n");
                dup2(node->fd[1], STDOUT_FILENO);
                close(node->fd[1]);
            }
    
            argv = (char**)malloc(sizeof(char*) * node->nArguments + 2);
    
            /* Set the executable (command) name */
            argv[i] = (char*) malloc (sizeof(char) * strlen(fullPath)+1);
            strcpy(argv[i++], fullPath);
    
            argumentNode* arg = node->argument;
            while( arg )
            {
                argv[i] = arg->value;
                arg = arg->next;
                i++;
            }
            argv[i] = NULL;
    
            /* Free the path returned before we change the forked image */
            //!! This is a fairly moot point here, all the memory will
            //!! be reclaimed on the exec anyway
            //free(fullPath);
            //fullPath = NULL;
    
            /* Create an image of the executable and run it */
            int n = execv(argv[0], argv);
            fprintf(stderr,"Exec returned %d\n", n);
        }
        else if(pid > 0)
        {
            printf("On parent\n");
        }
        else if(pid < 0)
        {
            printf("Fork has failed\n");
            //!! This should be done ALL the time on the parent
            //!! regardless of success in the fork.
            //free(fullPath);
            //fullPath = NULL;
        }
    
        // The parent, however it gets here, is not interested in these
        // descriptors, so close them.
        if( node->fd[0] > STDERR_FILENO )
        {
            fprintf(stderr,"P:pipe closed\n");
            close(node->fd[0]);
        }
        if( node->fd[1] > STDERR_FILENO )
        {
            fprintf(stderr,"P:pipe closed\n");
            close(node->fd[1]);
        }
    
        return pid;
    }
    
    int main ( ) {
      // do "ls | wc"
      commandNode   cmds[2] = {
        {
          { VAR_UNINITIALIZED, VAR_UNINITIALIZED },
          "/bin/ls"
        },
        {
          { VAR_UNINITIALIZED, VAR_UNINITIALIZED },
          "/usr/bin/wc"
        }
      };
    
      //First command isn't reading stdim, so this will be closed
      //cmds[0].fd[0] = STDIN_FILENO;
      cmds[1].fd[1] = STDOUT_FILENO;
    
      // pipefd[0] refers to the read end of the pipe.  pipefd[1] refers to the write end of the  pipe.
      int p[2];
      pipe(p);
      cmds[0].fd[1] = p[1];
      cmds[1].fd[0] = p[0];
    
      // spawn two processes
      int pid1 = genProcess( &cmds[0] );
      int pid2 = genProcess( &cmds[1] );
    
      // do some grim reaping
      printf("Parent waiting for %d %d\n",pid1,pid2);
      int status;
      pid1 = waitpid(-1,&status,0);
      if ( WIFEXITED(status) ) {
        printf("%d exited with normal status=%d\n", pid1, WEXITSTATUS(status) );
      } else {
        printf("Trouble for %d\n",pid1);
      }
      pid2 = waitpid(-1,&status,0);
      if ( WIFEXITED(status) ) {
        printf("%d exited with normal status=%d\n", pid2, WEXITSTATUS(status) );
      } else {
        printf("Trouble for %d\n",pid2);
      }
      return 0;
    }
    
    
    $ gcc -std=c99 foo.c
    $ ./a.out 
    On parent
    P:pipe closed
    cmd=/bin/ls
    C:Stdin Closed
    C:Stdout redirected
    On parent
    P:pipe closed
    Parent waiting for 3391 3392
    cmd=/usr/bin/wc
    C:Stdin redirected
         12      12      90
    3391 exited with normal status=0
    3392 exited with normal status=0
    If the parent doesn't close the pipes, then the child reading from the pipe ends up waiting for the parent to write something, after its sibling has died.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  7. #7
    Registered User
    Join Date
    Sep 2010
    Posts
    4
    Quote Originally Posted by Salem View Post
    Here's a hacked up test environment, and some results.
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <sys/types.h>
    #include <unistd.h>
    #include <sys/wait.h>
    
    #define VAR_UNINITIALIZED   -1
    
    typedef struct argumentNode {
      char     *value;
      struct argumentNode *next;
    } argumentNode;
    
    typedef struct commandNode {
      int           fd[2];
      char          *path;
      int           nArguments;
      int           value;
      argumentNode  *argument;
    } commandNode;
    
    char *getFullPath(char *path, int value) {
      return path;
    }
    
    int genProcess(commandNode* node)
    {
        //reservedNode* reserved = node->reservedL;
        int i = 0;
        int reservedCode = 0;
        char* fullPath = getFullPath(node->path, node->value);
        char** argv = NULL;
        pid_t pid;
    
        pid = fork();
    
        if( pid == 0 )
        {
            /* Set process to read */
            fprintf(stderr,"cmd=%s\n",fullPath);
            if( node->fd[0] == VAR_UNINITIALIZED ) {
              fprintf(stderr,"C:Stdin Closed\n");
              close(STDIN_FILENO);
            }
            if( node->fd[0] > STDERR_FILENO )
            {
                fprintf(stderr,"C:Stdin redirected\n");
                dup2(node->fd[0], STDIN_FILENO);
                close(node->fd[0]);
            }
    
            /* Set process to write */
            if( node->fd[1] == VAR_UNINITIALIZED ) {
              fprintf(stderr,"C:Stdout closed\n");
              close(STDOUT_FILENO);
            }
            if( node->fd[1] > STDERR_FILENO )
            {
                fprintf(stderr,"C:Stdout redirected\n");
                dup2(node->fd[1], STDOUT_FILENO);
                close(node->fd[1]);
            }
    
            argv = (char**)malloc(sizeof(char*) * node->nArguments + 2);
    
            /* Set the executable (command) name */
            argv[i] = (char*) malloc (sizeof(char) * strlen(fullPath)+1);
            strcpy(argv[i++], fullPath);
    
            argumentNode* arg = node->argument;
            while( arg )
            {
                argv[i] = arg->value;
                arg = arg->next;
                i++;
            }
            argv[i] = NULL;
    
            /* Free the path returned before we change the forked image */
            //!! This is a fairly moot point here, all the memory will
            //!! be reclaimed on the exec anyway
            //free(fullPath);
            //fullPath = NULL;
    
            /* Create an image of the executable and run it */
            int n = execv(argv[0], argv);
            fprintf(stderr,"Exec returned %d\n", n);
        }
        else if(pid > 0)
        {
            printf("On parent\n");
        }
        else if(pid < 0)
        {
            printf("Fork has failed\n");
            //!! This should be done ALL the time on the parent
            //!! regardless of success in the fork.
            //free(fullPath);
            //fullPath = NULL;
        }
    
        // The parent, however it gets here, is not interested in these
        // descriptors, so close them.
        if( node->fd[0] > STDERR_FILENO )
        {
            fprintf(stderr,"P:pipe closed\n");
            close(node->fd[0]);
        }
        if( node->fd[1] > STDERR_FILENO )
        {
            fprintf(stderr,"P:pipe closed\n");
            close(node->fd[1]);
        }
    
        return pid;
    }
    
    int main ( ) {
      // do "ls | wc"
      commandNode   cmds[2] = {
        {
          { VAR_UNINITIALIZED, VAR_UNINITIALIZED },
          "/bin/ls"
        },
        {
          { VAR_UNINITIALIZED, VAR_UNINITIALIZED },
          "/usr/bin/wc"
        }
      };
    
      //First command isn't reading stdim, so this will be closed
      //cmds[0].fd[0] = STDIN_FILENO;
      cmds[1].fd[1] = STDOUT_FILENO;
    
      // pipefd[0] refers to the read end of the pipe.  pipefd[1] refers to the write end of the  pipe.
      int p[2];
      pipe(p);
      cmds[0].fd[1] = p[1];
      cmds[1].fd[0] = p[0];
    
      // spawn two processes
      int pid1 = genProcess( &cmds[0] );
      int pid2 = genProcess( &cmds[1] );
    
      // do some grim reaping
      printf("Parent waiting for %d %d\n",pid1,pid2);
      int status;
      pid1 = waitpid(-1,&status,0);
      if ( WIFEXITED(status) ) {
        printf("%d exited with normal status=%d\n", pid1, WEXITSTATUS(status) );
      } else {
        printf("Trouble for %d\n",pid1);
      }
      pid2 = waitpid(-1,&status,0);
      if ( WIFEXITED(status) ) {
        printf("%d exited with normal status=%d\n", pid2, WEXITSTATUS(status) );
      } else {
        printf("Trouble for %d\n",pid2);
      }
      return 0;
    }
    
    
    $ gcc -std=c99 foo.c
    $ ./a.out 
    On parent
    P:pipe closed
    cmd=/bin/ls
    C:Stdin Closed
    C:Stdout redirected
    On parent
    P:pipe closed
    Parent waiting for 3391 3392
    cmd=/usr/bin/wc
    C:Stdin redirected
         12      12      90
    3391 exited with normal status=0
    3392 exited with normal status=0
    If the parent doesn't close the pipes, then the child reading from the pipe ends up waiting for the parent to write something, after its sibling has died.
    Hi Salem,

    It worked here. I was really forgetting to close the pipes in the parent. Besides that there was a problem with the way I was waiting for the children to finish, which was causing zombies processes. This was fixed.

    Thanks for the help and have a nice week!
    Caio

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 4
    Last Post: 10-14-2009, 04:44 PM
  2. Replies: 3
    Last Post: 06-02-2009, 06:13 PM
  3. Trying to understand fork, dup2, and execvp
    By Unclejunebug in forum C Programming
    Replies: 0
    Last Post: 04-29-2009, 07:04 PM
  4. fork() and pipe()
    By scioner in forum C Programming
    Replies: 3
    Last Post: 06-13-2008, 11:32 PM
  5. fork and pipe
    By tbarsness in forum C Programming
    Replies: 3
    Last Post: 10-22-2007, 12:40 PM