16

I am struggling with process creation and piping the child process' output into a string of the parent process. I got it working on Windows (using CreatePipe and CreateProcess and ReadFile), but can't seem to get the exact analog on Unix to work. This is my code:

#include <spawn.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/wait.h>
#include <iostream>
#include <string>
#include <vector>
using namespace std;

int main()
{
  int exit_code;
  int cout_pipe[2];
  int cerr_pipe[2];
  posix_spawn_file_actions_t action;

  if(pipe(cout_pipe) || pipe(cerr_pipe))
    cout << "pipe returned an error.\n";

  posix_spawn_file_actions_init(&action);
  posix_spawn_file_actions_addclose(&action, cout_pipe[0]);
  posix_spawn_file_actions_addclose(&action, cerr_pipe[0]);
  posix_spawn_file_actions_adddup2(&action, cout_pipe[1], 1);
  posix_spawn_file_actions_adddup2(&action, cerr_pipe[1], 2);

  posix_spawn_file_actions_addclose(&action, cout_pipe[1]);
  posix_spawn_file_actions_addclose(&action, cerr_pipe[1]);

  vector<string> argmem = {"bla"};
  vector<char*> args = {&argmem[0][0], nullptr}; // I don't want to call new.

  pid_t pid;
  if(posix_spawnp(&pid, "echo", &action, NULL, &args[0], NULL) != 0)
    cout << "posix_spawnp failed with error: " << strerror(errno) << "\n";
  //close(cout_pipe[0]);
  //close(cerr_pipe[0]);

  close(cout_pipe[1]);
  close(cerr_pipe[1]);

  waitpid(pid,&exit_code,0);
  cout << "exit code: " << exit_code << "\n";

  // Read from pipes
  const size_t buffer_size = 1024;
  string buffer;
  buffer.resize(buffer_size);
  ssize_t bytes_read = read(cout_pipe[0], &buffer[0], buffer_size);
  while ((bytes_read = read(cout_pipe[0], &buffer[0], buffer_size)) > 0)
  {
    cout << "read " << bytes_read << " bytes from stdout.\n";
    cout << buffer.substr(0, static_cast<size_t>(bytes_read)+1) << "\n";
    bytes_read = read(cout_pipe[0], &buffer[0], buffer_size);
  }
  if(bytes_read == -1)
    cout << "Failure reading from stdout pipe.\n";
  while ((bytes_read = read(cerr_pipe[0], &buffer[0], buffer_size)) > 0)
  {
    cout << "read " << bytes_read << " bytes from stderr.\n";
    cout << buffer.substr(0, static_cast<size_t>(bytes_read)+1) << "\n";
    bytes_read = read(cout_pipe[0], &buffer[0], buffer_size);
  }
  if(bytes_read == -1)
    cout << "Failure reading from stderr pipe.\n";

  posix_spawn_file_actions_destroy(&action);
}

The output is:

exit code: 0

So I suppose everything is working except the actual piping. What is wrong here? I also wonder if there is a way to read the piped bytes in a waitpid loop, but when I try that, the parent process hangs infinitely.

rubenvb
  • 74,642
  • 33
  • 187
  • 332
  • I had never heard of `posix_spawnp`. What's wrong with good old `popen`? What an interesting syscall/library function it is. Will remember this :) – sehe Dec 15 '12 at 19:44
  • 1
    `popen` doesn't have the required flexibility: AFAICT I can't redirect stderr with that. It is also the function that most closely resembles the `CreateProcess` API. Heck, MSDN even has `_spawnvp` which resembles `posix_spawnp`, but as I said, my `CreateProcess` code is basically working fine. It's the Unix side that's currently not cooperating :( – rubenvb Dec 16 '12 at 11:12
  • 3
    I added the C tag to raise interest. Although the code is technically C++, the important bits (ie the piping and spawning) is pure C. – rubenvb Dec 16 '12 at 12:54

1 Answers1

28

posix_spawn is interesting and useful, which makes this question worth necromancing -- even if it is no longer relevant to the OP.

There are some significant bugs in the code as posted. I suspect that some of these were the result of hacking in desperation, but I don't know which was the original bug:

  1. The args array does not include the argv[0] that would represent the executable name. This results in the echo program never seeing the intended argv[1] ("bla").
  2. The read() function is called from different places in a way that just doesn't make sense. A correct way to do this would be to only call read as part of the control expression for the while loops.
  3. waitpid() is called before reading from the pipes. This prevents the I/O from completing (in non-trivial cases at least).
  4. A more subtle issue with this code is that attempts to read all of the child's stdout before reading anything from stderr. In principle, this could cause the child to block while attempting to write to stderr, thus preventing the program from completing. Creating an efficient solution to this is more complicated as it requires that you can read from whichever pipe has available data. I used poll() for this. Another approach would be to use multiple threads.

Additionally, I have used sh (the command shell, i.e. bash) as the child process. This provides a great deal of additional flexibility, such as running a pipeline instead of a single executable. In particular, though, using sh provides the simple convenience of not having to manage the parsing of the command-line.

/*BINFMTCXX: -std=c++11 -Wall -Werror
*/

#include <spawn.h> // see manpages-posix-dev
#include <poll.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/wait.h>
#include <iostream>
#include <string>
#include <vector>
using namespace std;

int main()
{
  int exit_code;
  int cout_pipe[2];
  int cerr_pipe[2];
  posix_spawn_file_actions_t action;

  if(pipe(cout_pipe) || pipe(cerr_pipe))
    cout << "pipe returned an error.\n";

  posix_spawn_file_actions_init(&action);
  posix_spawn_file_actions_addclose(&action, cout_pipe[0]);
  posix_spawn_file_actions_addclose(&action, cerr_pipe[0]);
  posix_spawn_file_actions_adddup2(&action, cout_pipe[1], 1);
  posix_spawn_file_actions_adddup2(&action, cerr_pipe[1], 2);

  posix_spawn_file_actions_addclose(&action, cout_pipe[1]);
  posix_spawn_file_actions_addclose(&action, cerr_pipe[1]);

//string command = "echo bla"; // example #1
  string command = "pgmcrater -width 64 -height 9 |pgmtopbm |pnmtoplainpnm";
  string argsmem[] = {"sh","-c"}; // allows non-const access to literals
  char * args[] = {&argsmem[0][0],&argsmem[1][0],&command[0],nullptr};

  pid_t pid;
  if(posix_spawnp(&pid, args[0], &action, NULL, &args[0], NULL) != 0)
    cout << "posix_spawnp failed with error: " << strerror(errno) << "\n";

  close(cout_pipe[1]), close(cerr_pipe[1]); // close child-side of pipes

  // Read from pipes
  string buffer(1024,' ');
  std::vector<pollfd> plist = { {cout_pipe[0],POLLIN}, {cerr_pipe[0],POLLIN} };
  for ( int rval; (rval=poll(&plist[0],plist.size(),/*timeout*/-1))>0; ) {
    if ( plist[0].revents&POLLIN) {
      int bytes_read = read(cout_pipe[0], &buffer[0], buffer.length());
      cout << "read " << bytes_read << " bytes from stdout.\n";
      cout << buffer.substr(0, static_cast<size_t>(bytes_read)) << "\n";
    }
    else if ( plist[1].revents&POLLIN ) {
      int bytes_read = read(cerr_pipe[0], &buffer[0], buffer.length());
      cout << "read " << bytes_read << " bytes from stderr.\n";
      cout << buffer.substr(0, static_cast<size_t>(bytes_read)) << "\n";
    }
    else break; // nothing left to read
  }

  waitpid(pid,&exit_code,0);
  cout << "exit code: " << exit_code << "\n";

  posix_spawn_file_actions_destroy(&action);
}
Community
  • 1
  • 1
Brent Bradburn
  • 51,587
  • 17
  • 154
  • 173
  • I didn't thoroughly button this up. There's plenty of room for better error checking and resource cleanup. – Brent Bradburn Dec 06 '14 at 17:24
  • 1
    Why didn't you pass in just args but did &args[0] to posix_spawn()'s 5th argument? – Dula Jan 07 '16 at 02:28
  • 1
    @Dula: The form I used is more general and more clear to me. It works for pointers, but also for class-based array managers such as `std::vector` and `std::string`. I use that form in several places, including `&buffer[0]`, which would not have worked as just `buffer`. – Brent Bradburn Jan 07 '16 at 15:38
  • 1
    here is a snippet that instead of poll() uses select(), https://github.com/pixley/InvestigativeProgramming/blob/114b698339fb0243f50cf5bfbe5d5a701733a125/test_spawn_pipe.cpp – neoneye May 28 '16 at 19:02