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?

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.