title: Bad file descriptor in C with pipe(), dup2(), and fork()
body: <p>I'm having trouble with file descriptors in C.</p>
  <p>I have a primary method (called <em>executeShell()</em>) that calls another method called <em>getProgramParameters</em>. <em>getProgramParameters</em> parses a few basic bash commands (e.g. &quot;ls &gt; output&quot;) and returns three things: the command (&quot;ls&quot; in the example), the input file if applicable (there isn't one in the example), and an output file if applicable (&quot;output&quot; in the example). It's a simplified redirection parser that also parses pipes, with the following exceptions: multiple pipes/redirections are not allowed, and it returns an error if you try to provide multiple inputs or outputs (e.g. &quot;ls &gt; output | cat&quot; is invalid because of multiple outputs, and &quot;ls | cat &lt; output&quot; is invalid because of multiple inputs). If a pipe is encountered, <em>getProgramParameters</em> is called again on the command after the pipe. The sets of parameters are called <strong>firstParams</strong> and <strong>secondParams</strong>, respectively.</p>
  <p>In <em>executeShell()</em>, if I parse two sets of parameters (i.e. I encountered a pipe), I call <em>pipe()</em> once and <em>fork()</em> twice to make two children (<strong>child_a</strong> and <strong>child_b</strong> respectively) that will be running the commands proper. I pipe the first program's (<strong>child_a's</strong>) write to the second program's (<strong>child_b's</strong>) read and close the unnecessary ends in each. I call another method called <em>runCommand()</em> in each child, which takes the values from <strong>first/secondParams</strong> and calls its own <em>fork()</em> to run the command (ls, cat, etc.) itself.</p>
  <p>I explain all of this because somewhere in this process I'm getting bad file descriptors, and I can't tell why. The file descriptors for the input/output files of the actual commands (e.g. &quot;cat &lt; input &gt; output&quot;) are generated by <em>getProgramParameters()</em>, and to complicate matters further I create stdin/out copies in both <em>executeShell()</em> and <em>runCommand()</em> so I can restore them after the children processes <em>dup2()</em> the input/output accordingly. I tried to output them to see what's going on, and I realized that there are file descriptors that are the same when they shouldn't be. I can't tell why this is happening.</p>
  <p>The output for &quot;ls &gt; output&quot; (no piping):</p>
  <pre><code>sh# ls &gt; output
  opened file for output with fd = 3
  format: program &gt; file
  first params
    ls
    -1
    3
  tokenizer null for firstParams, no second params
  running program
  stdinCopy: 3
  stdoutCopy: 4
  output fd = 3, duping that to STDOUT
  Invalid: Error in duping output to STDOUT: Bad file descriptor
  error in runCommand
  sh#
  </code></pre>
  <p>and the output for &quot;ls | cat &gt; output&quot;:</p>
  <pre><code>sh# ls | cat &gt; output
  pipe found
  first params
    ls
    -1
    -1
  opened file for output with fd = 3
  format: program &gt; file
  second params
    cat
    -1
    3
  stdinCopy (child_b): 5
  running second process
  stdoutCopy (child_a): 5
  running program
  stdinCopy: 4
  stdoutCopy: 6
  output fd = 3, duping that to STDOUT
  Invalid: Error in duping output to STDOUT: Bad file descriptor
  Invalid: second program execution failed: Bad file descriptor
  sh#
  </code></pre>
  <p>as a side note, don't worry about tokenizer, it's just a class that helps parse and indicates whether or not there's a pipe.</p>
  <p>I apologize on the complexity of this and the methods in advance, I just don't understand why I'm getting the same file descriptor numbers, and all these methods are involved:</p>
  <p><em>executeShell</em>:</p>
  <pre><code>void executeShell() {
    char minishell[] = &quot;sh# &quot;;
    writeToStdout(minishell);
  
    TOKENIZER *tokenizer;
    tokenizer = getInput();
    if (tokenizer == NULL) {
      printf(&quot;%s\n&quot;, &quot;Exiting...&quot;);
      exit(EXIT_SUCCESS);
    }
    if (*tokenizer-&gt;str == '\0') return;
  
    // when reading the first params,
    // no output is allowed if pipe
    PARAMS* firstParams;
    PARAMS* secondParams;
    firstParams = getProgramParameters(tokenizer);
    if (firstParams == NULL) {
      writeToStdout(&quot;first params null\n&quot;);
      return;
    }
    printf(&quot;%s\n&quot;, &quot;first params&quot;);
    printf(&quot;  %s\n&quot;, firstParams-&gt;command);
    printf(&quot;  %d\n&quot;, firstParams-&gt;input);
    printf(&quot;  %d\n&quot;, firstParams-&gt;output);
    if (firstParams-&gt;tokenizer != NULL) {
      secondParams = getProgramParameters(firstParams-&gt;tokenizer);
      if (secondParams != NULL) {
        printf(&quot;%s\n&quot;, &quot;second params&quot;);
        printf(&quot;  %s\n&quot;, secondParams-&gt;command);
        printf(&quot;  %d\n&quot;, secondParams-&gt;input);
        printf(&quot;  %d\n&quot;, secondParams-&gt;output);
  
        // at this point secondParams' tokenizer must be null because we only take one pipe
        if (secondParams-&gt;tokenizer != NULL) {
          printf(&quot;%s\n&quot;, &quot;Invalid: multiple pipes&quot;);
          return;
        }
        if (secondParams-&gt;input != -1) {
          printf(&quot;%s\n&quot;, &quot;Invalid: multiple inputs&quot;);
          return;
        }
        // create pipe, fork, etc.
        int fd[2], status;
        pid_t child_a, child_b;
        // char firstOutput[1024];
  
        if (pipe(fd) &lt; 0) {
          printf(&quot;%s\n&quot;, &quot;Invalid: pipe error&quot;);
          return;
        }
        if ((child_a = fork()) &lt; 0) {
          printf(&quot;%s\n&quot;, &quot;Invalid: fork error&quot;);
          return;
        } else if (child_a == 0) {
          // child_a
          // connecting write end (1) to stdout
          int stdoutCopy = dup(STDOUT_FILENO);
          printf(&quot;stdoutCopy (child_a): %d\n&quot;, stdoutCopy);
          if (dup2(fd[1], STDOUT_FILENO) == -1) {
            perror(&quot;Invalid: Error in duping output to pipe write&quot;);
            exit(EXIT_FAILURE);
          }
          // close read end (0)
          close(fd[0]);
          printf(&quot;%s\n&quot;, &quot;running first process&quot;);
          if (runCommand(firstParams-&gt;command, firstParams-&gt;input, -1) != 0) {
            perror(&quot;Invalid: first program execution failed&quot;);
            dup2(stdoutCopy, STDOUT_FILENO);
            exit(EXIT_FAILURE);
          }
          if (dup2(STDOUT_FILENO, stdoutCopy) == -1) {
            perror(&quot;Invalid: Error in restoring STDOUT&quot;);
            exit(EXIT_FAILURE);
          }
          exit(EXIT_SUCCESS);
        } else {
          // parent
          child_b = fork();
          if (child_b &lt; 0) {
            printf(&quot;%s\n&quot;, &quot;Invalid: fork error&quot;);
            return;
          } else if (child_b == 0) {
            // child_b
            // connect read end (0) to stdin
            int stdinCopy = dup(STDIN_FILENO);
            printf(&quot;stdinCopy (child_b): %d\n&quot;, stdinCopy);
            if (dup2(fd[0], STDIN_FILENO) == -1) {
              perror(&quot;Invalid: Error in duping input to pipe read&quot;);
              exit(EXIT_FAILURE);
            }
            // close write end (1)
            close(fd[1]);
            printf(&quot;%s\n&quot;, &quot;running second process&quot;);
            if (runCommand(secondParams-&gt;command, -1, secondParams-&gt;output) != 0) {
              perror(&quot;Invalid: second program execution failed&quot;);
              dup2(stdinCopy, STDIN_FILENO);
              exit(EXIT_FAILURE);
            }
            if (dup2(STDIN_FILENO, stdinCopy) == -1) {
              perror(&quot;Invalid: Error in restoring STDIN&quot;);
              exit(EXIT_FAILURE);
            }
            exit(EXIT_SUCCESS);
          } else {
            // back to parent
            do {
              if (wait(&amp;status) == -1) {
                perror(&quot;Error in child process termination&quot;);
                exit(EXIT_FAILURE);
                return;
              }
            } while (!WIFEXITED(status) &amp;&amp; !WIFSIGNALED(status));
            waitpid(child_a, &amp;status, 0);
            waitpid(child_b, &amp;status, 0);
          }
        }
        free_params(firstParams);
        free_params(secondParams);
        return;
      } else {
        printf(&quot;%s\n&quot;, &quot;no second params&quot;);
      }
    } else {
      writeToStdout(&quot;tokenizer null for firstParams, no second params\n&quot;);
      if (runCommand(firstParams-&gt;command, firstParams-&gt;input, firstParams-&gt;output) != 0) {
        printf(&quot;%s\n&quot;, &quot;error in runCommand&quot;);
      }
      free_params(firstParams);
      return;
    }
    // when reading the second params,
    // no input is allowed
    free_tokenizer(tokenizer);
    free_params(firstParams);
  }
  </code></pre>
  <p>here's the bit from <em>getProgramParameters</em> that generates the file descriptor for &quot;output&quot; (where tok2 is just the second token such as &quot;tok1 &gt; tok2&quot;, and direction is the &quot;&gt;&quot; or &quot;&lt;&quot;):</p>
  <pre><code>if (strcmp(&quot;&gt;&quot;, direction) == 0) {
      output = open(tok2, O_WRONLY | O_TRUNC | O_CREAT, 0644);
      if (output == -1) {
        printf(&quot;%s\n&quot;, &quot;Invalid: second token file invalid&quot;);
        goto cleanup;
      } else {
        printf(&quot;%s%d\n&quot;, &quot;opened file for output with fd = &quot;, output);
      }
  </code></pre>
  <p>the relevant code from <em>runCommand</em>:</p>
  <pre><code>printf(&quot;%s\n&quot;, &quot;running program&quot;);
    int status;
    childPid = fork();
  
    if (childPid &lt; 0) {
      perror(&quot;Invalid: Error in creating child process&quot;);
      return -1;
    }
  
    if (childPid == 0) {
      // child
      // read in from file 
      // printf(&quot;%s\n&quot;, &quot;test&quot;);
      int stdinCopy = dup(STDIN_FILENO);
      printf(&quot;stdinCopy: %d\n&quot;, stdinCopy);
      if (file != -1) {
        printf(&quot;%s\n&quot;, &quot;file not null, duping file to STDIN&quot;);
        if (dup2(STDIN_FILENO, file) == -1) {
          perror(&quot;Invalid: Error in duping file to STDIN&quot;);
          return -1;
        }
      }
      // printf(&quot;%s\n&quot;, &quot;test&quot;);
      int stdoutCopy = dup(STDOUT_FILENO);
      printf(&quot;stdoutCopy: %d\n&quot;, stdoutCopy);
      if (output != -1) {
        printf(&quot;%s%d%s\n&quot;, &quot;output fd = &quot;, output, &quot;, duping that to STDOUT&quot;);
        if (dup2(STDOUT_FILENO, output) != 0) {
          perror(&quot;Invalid: Error in duping output to STDOUT&quot;);
          return -1;
        }
      }
      // printf(&quot;%s\n&quot;, &quot;test&quot;);
      if (execvp(args[0], args) != 0) {
        // un dup
        if (dup2(stdinCopy, STDIN_FILENO) == -1 || dup2(stdoutCopy, STDOUT_FILENO) == -1) {
          perror(&quot;Invalid: Error in restoring STDIN/STDOUT&quot;);
          return -1;
        }
        close(stdinCopy);
        close(stdoutCopy);
        perror(&quot;Invalid: Error in execvp&quot;);
        // printf(&quot;%s\n&quot;, &quot;test&quot;);
        output = -1;
        return -1;
      }
      // printf(&quot;%s\n&quot;, &quot;test&quot;);
      if (dup2(stdinCopy, STDIN_FILENO) == -1 || dup2(stdoutCopy, STDOUT_FILENO) == -1) {
        perror(&quot;Invalid: Error in restoring STDIN/STDOUT&quot;);
        return -1;
      }
      close(stdinCopy);
      close(stdoutCopy);
      // printf(&quot;%s\n&quot;, &quot;process run succesfully&quot;);
    } else {
      // parent
      do {
        if (wait(&amp;status) == -1) {
          perror(&quot;Error in child process termination&quot;);
          exit(EXIT_FAILURE);
          return -1;
        }
      } while (!WIFEXITED(status) &amp;&amp; !WIFSIGNALED(status));
      // reset childPid
      childPid = 0;
    }
    for (int i=0; i&lt;numArgs; i++) {
      free(args[i]);
    }
    return 0;
  }
  </code></pre>
  <p>I'm also getting a zombie process dependent on the command, and I'm not sure why that's happening, either. I tried to call <em>wait()</em> appropriately and ensure each process exited regardless...</p>
  <p>Update: I have commented out the stdin/out copies, as I suspected they may be unnecessary. I'm still getting the file descriptor errors, though. I also tried to provide a copy of the stdin/out to the <em>runCommand()</em> method suspecting that I was closing the standard versions as a commenter pointed out, this doesn't work either.</p>
  <p>Second update: Changed all &quot;file&quot; variables names to &quot;input&quot; for consistency, called correct <em>_exit</em> in child in <em>runCommand()</em>, no longer getting zombies. Tried changing my PARAMS class to take <em>input</em> and <em>output</em> as integer pointers instead (thinking that maybe there was something special about the integer returned from an <em>open</em> call for the file descriptors), but it turns out that children from <em>fork</em> really don't like the pointer within the class. I was getting this (changed some prints too):</p>
  <pre><code>sh# ls &gt; output
  initializing input as -1
  initializing output as 3
  runCommand seeing input as -1 and output as 3
  input fd is 1484746160, duping that to STDIN (0)
  Invalid: Error in duping input to STDIN
  Bad file descriptor
  sh# Exiting...
  </code></pre>
  <p>In sum, still getting &quot;bad file descriptor&quot; even if I'm only calling <code>dup2(output, STDOUT_FILENO)</code> (not shown above).</p>
  <p>Update: I just realized that my dup2 check should see if it's -1, not anything but 0. dup2 will return the new file descriptor.</p>
  <p>I fixed the problem by calling open() within the child itself. I could not for the life of me figure out why the file descriptor wasn't working as a straight integer - maybe once it exited the method or forked it tried to free up resources or something? Idk, but it forced me to restructure a lot, but it worked in the end.</p>