Blank string on redirection with dup2

Question

So the string that I believe dup2 should be redirecting output into is blank. I've tried a couple of different methods that I've found here but haven't been able to get it to work. I believe my problem is reading/accessing the file descriptor.

char results[4096];

That is the variable that I'm trying to write the output into.

void runCommand(char **args) {
  pid_t pid;
  pid = fork(); //forking because if I don't the program will end after execvp runs

  int pipefd[2];
  int child_process_output_fd;

  if(pipe(pipefd) == -1){
    printf("A piping error has occurred");
  }

  if(pid < 0){
    printf("Error Forking");
  } else if (pid == 0) {

    close(pipefd[0]);
    dup2(pipefd[1], STDOUT_FILENO);

    if(execvp(args[0], args) == -1){
      printf("An error has occurred running the command\n");
    }
    exit(EXIT_SUCCESS);
  } else {
    close(pipefd[1]);


    wait(&pid);
    read(pipefd[0], results, BUFFER_SIZE);
    printf("finished child comand\n");
    printf("The output is: %s\n", results);

  }
}

Answer 1

The problems in descending order of severity:

• You need to create the pipe before you fork. You should be creating one pipe, writing to one end of it, and reading from the other end of it. Instead, you are current creating two pipes, writing to one, and reading from the other entirely differently pipe.

• read doesn't return a NUL-terminated string, so printf("The output is: %s\n", results); results in undefined behaviour.

• Calling read after waitpid is wrong. It can result in a deadlock. You should read before reaping the child.

• read(pipefd[0], results, BUFFER_SIZE); is not good enough. You need to read until read returns 0 or an error.

• wait(&pid); is wrong. It should be waitpid(pid, NULL, 0); or int status; waitpid(pid, &status, 0);.

• Error messages should be sent to stderr, not stdout. Use fprintf(stderr, ...) instead of printf(...). You can also use perror to automatically add the appropriate error message represented by errno.

• execvp doesn't return on success —it replaces the program being executed by the process with a different program— so exit(EXIT_SUCCESS); is never reached.

Fixed:

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>


// Reads from file descriptor fd until EOF.
// *buf_ptr is assigned a pointer to the buffer containing what was read or NULL.
// *size_ptr is assigned the total number of bytes read.
//
// On success, returns 0.
// On error, returns -1 and sets errno.
//
// Whether successful or not, everything read
// will be accessible via the pointer in *buf_ptr.
// If *buf_ptr is not NULL, it needs to be freed.

int read_all(int fd, char **buf_ptr, size_t *size_ptr) {
   const size_t BLOCK_SIZE = 64*1024;

   *size_ptr = 0;
   *buf_ptr = NULL;

   while (1) {
      void *new_buf = realloc(*buf_ptr, *size_ptr + BLOCK_SIZE);
      if (!new_buf) {
         *buf_ptr = realloc(*buf_ptr, *size_ptr);  // Free unused space.
         return 0;
      }

      *buf_ptr = new_buf;
      char *p = ((char*)*buf_ptr) + *size_ptr;
      ssize_t bytes_read = read(fd, p, BLOCK_SIZE);
      if (bytes_read <= 0) {
         *buf_ptr = realloc(*buf_ptr, *size_ptr);  // Free unused space.
         return bytes_read;
      }

      *size_ptr += bytes_read;
   }
}


// Writes size bytes to file descriptor fd.
// If written isn't NULL, *total_written is
// assigned the amount actually written.
// This will only be different than amount
// requested on error.
//
// On success, returns 0.
// On error, returns -1 and sets errno.

int write_all(int fd, char *buf, size_t size, size_t *total_written_ptr) {
   if (total_written_ptr)
      *total_written_ptr = 0;

   while (size > 0) {
      ssize_t written = write(fd, buf, size);
      if (written < 0)
         return -1;

      size -= written;
      buf += written;

      if (total_written_ptr)
         *total_written_ptr = 0;
   }

   return 0;
}


void run_command(char *const argv[]) {
   int pipefd[2];
   if (pipe(pipefd) < 0) {
      perror("Error creating pipe");
      exit(EXIT_FAILURE);
   }

   pid_t pid = fork();
   if (pid < 0) {
      perror("Error forking");
      exit(EXIT_FAILURE);
   }

   if (pid == 0) {
      // Child.
      close(pipefd[0]);
      dup2(pipefd[1], STDOUT_FILENO);

      execvp(argv[0], argv);
      perror("Executing child");
      exit(EXIT_FAILURE);
   }

   close(pipefd[1]);

   char *results;
   size_t size;
   if (read_all(pipefd[0], &results, &size) < 0) {
      perror("Error reading");
      free(results);
      return;
   }

   int status;
   waitpid(pid, &status, 0);
   if (WIFSIGNALED(status)) {
      fprintf(stderr, "Warning: Child was killed by signal %d\n", WTERMSIG(status));
   }
   else if (WEXITSTATUS(status) != 0) {
      fprintf(stderr, "Warning: Child exited with error %d\n", WEXITSTATUS(status));
   }

   printf("Client returned:\n");
   fflush(stdout);
   write_all(STDOUT_FILENO, results, size, NULL);
   free(results);
}


int main(void) {
   char *cmd[] = { "ls", "-1", NULL };
   run_command(cmd);
}

Output:

$ gcc -Wall -Wextra -pedantic a.c -o a && a
Client returned:
a
a.c