Running a shell command and getting output

Question

This is the code I'm currently using to run a shell command on the current machine, intended for use only with GNU/Linux at the moment:

std::string getCmdOutput(const std::string& mStr)
{
 std::string result, file;
 FILE* pipe{popen(mStr.c_str(), "r")};
 char buffer[256];
 while(fgets(buffer, sizeof(buffer), pipe) != NULL)
 {
 file = buffer;
 result += file.substr(0, file.size() - 1);
 }
 pclose(pipe);
 return result;
}

Example use:

getCmdOutput(R"( sudo pacman -Syyuu )");
auto output(getCmdOutput(R"( echo /etc/pacman.conf )"));

I use this when making simple script-like C++ programs (mostly for personal use).

Is this the "correct" way of doing it? What can be improved?

• YoungJohn: In C++ I believe it is a bit more idiomatic to use fstream to open files instead of using FILE* which is a C API, but there doesn't appear to be anything wrong with what you're doing in that regard.
• Loki Astari: The pipe opens up a two way pipe so you can write to standard input of the application you started. I would rather wrap it up inside an stream so you can read/write from/to the stream and it ends up going to the application.
• Vittorio Romeo: @YoungJohn: Could you please show me an example of using std::ifstream to get the output of a shell command? I tried using std::getline but I do not receive any output.
• David Doria: I would also like to know how to get the bash output into an ifstream.

Answer 1

Skipping the C++ specific parts, I would encourage you to do more for the error handling...

• check the return value of popen!!! (nullptr for the FILE*)
• check the return-value of pclose() If the pipe failed for any reason (including if there was an exit-code from the called script), the pclose() will be set to an error.
• you are not trapping the STDERR of the process you call, which means you are not capturing/buffering the output, and you may end up with crap on your console, or missing data you expect, or both.

Answer 2

Prefer comparing pointers to nullptr instead of to NULL in C++11. nullptr exists for exactly that reason. The NULL macro is not portable. While it exists in many different environments there is no guarantee how it will be implemented or what it is intended to be used for. It may be implemented as 0, '0円', or as some pointer type, or anything else, and every different environment may define it differently or not at all. It is non-standard. nullptr is defined by the standard and all C++11 standard compliant compilers must support it.