Search code examples
c++winapireadfilecreateprocess

C++ WInApi ReadFile reads nothing and reports error 109 ERROR_BROKEN_PIPE


I have been trying to call adb from a C++ app and read its output, without success. adb is present on PATH.

The ReadFile call never reads anything, even if I wait.

GetLastError() returns 109 ERROR_BROKEN_PIPE.

I have tried all the solutions proposed in all existing StackOverflow questions on this matter, without any success.

Even trying to capture output from cmd /c echo Hello World doesn't work.

Code:

#include <array>
#include <string>
#include <windows.h>

int main() {
    std::string adbConnectCommand = "adb connect localhost:5555";

    STARTUPINFO startupInfo;
    SECURITY_ATTRIBUTES secAttr;
    PROCESS_INFORMATION procInfo;

    HANDLE stdoutReadHandle = NULL;
    HANDLE stdoutWriteHandle = NULL;

    ZeroMemory(&secAttr, sizeof(secAttr));
    secAttr.nLength = sizeof(SECURITY_ATTRIBUTES);
    secAttr.bInheritHandle = TRUE;
    secAttr.lpSecurityDescriptor = NULL;

    // Create a pipe for the child process's STDOUT
    if (!CreatePipe(&stdoutReadHandle, &stdoutWriteHandle, &secAttr, 0))
        return 2; // error

    // Ensure the read handle to the pipe for STDOUT is not inherited
    if (!SetHandleInformation(stdoutReadHandle, HANDLE_FLAG_INHERIT, 0))
        return 2; // error

    ZeroMemory(&startupInfo, sizeof(startupInfo));
    startupInfo.cb = sizeof(startupInfo);
    startupInfo.hStdError = &stdoutWriteHandle;
    startupInfo.hStdOutput = &stdoutWriteHandle;
    startupInfo.dwFlags |= STARTF_USESTDHANDLES;

    ZeroMemory(&procInfo, sizeof(procInfo));

    // Start the child process
    if (CreateProcessA(NULL,                // No module name (use command line)
        (TCHAR*)adbConnectCommand.c_str(),  // Command line
        NULL,                               // Process handle not inheritable
        NULL,                               // Thread handle not inheritable
        TRUE,                               // Set handle inheritance
        0,                                  // No creation flags
        NULL,                               // Use parent's environment block
        NULL,                               // Use parent's starting directory 
        &startupInfo,                       // Pointer to STARTUPINFO structure
        &procInfo)                          // Pointer to PROCESS_INFORMATION structure
        )
    {
        // Close the handles we don't need so we can read from stdoutReadHandle
        CloseHandle(procInfo.hProcess);
        CloseHandle(procInfo.hThread);
        CloseHandle(stdoutWriteHandle);

        std::array<char, 512> buffer;
        std::string adbConnectOutput;

        while(ReadFile(stdoutReadHandle, buffer.data(), buffer.size(), NULL, NULL))
            adbConnectOutput += buffer.data();

        CloseHandle(stdoutReadHandle);

        if (adbConnectOutput.find("connected to localhost:5555") == std::string::npos)
            return 1; // not found

        m_Connected = true;
        return 0; // success
    }

    return 2; // error
}

Solution

  • In this code:

    startupInfo.hStdError = &stdoutWriteHandle;
    startupInfo.hStdOutput = &stdoutWriteHandle;
    

    HANDLE is defined as a void*, so any pointer can be assigned to it. But not every pointer is a valid HANDLE value. In this case, stdoutWriteHandle is a HANDLE variable and can be assigned as-is to hStdError and hStdOutput, so you need to drop the &s as they don't belong there, eg:

    startupInfo.hStdError = stdoutWriteHandle;
    startupInfo.hStdOutput = stdoutWriteHandle;
    

    That being said, there are some other problems with your code.

    In this code:

    // Start the child process
    if (CreateProcessA(NULL,                // No module name (use command line)
        (TCHAR*)adbConnectCommand.c_str(),  // Command line
        ...
    

    TCHAR* is the wrong type to cast to here. CreateProcessA() expects LPSTR (aka char*) instead, but TCHAR* may or may not be char* depending on your project configuration. So use the proper type, eg:

    CreateProcessA(NULL,
        (LPSTR)adbConnectCommand.c_str(),
        ...
    

    However, you should be using the C++-style const_cast instead of a C-style cast, eg:

    CreateProcessA(NULL,
        const_cast<LPSTR>(adbConnectCommand.c_str()),
        ...
    

    Alternatively, use std::string::data() (C++17 and later) or std::string::operator[] instead, eg:

    CreateProcessA(NULL,
        adbConnectCommand.data(),
        ...
    
    CreateProcessA(NULL,
        &adbConnectCommand[0],
        ...
    

    And, in this code:

    while(ReadFile(stdoutReadHandle, buffer.data(), buffer.size(), NULL, NULL))
        adbConnectOutput += buffer.data();
    

    The data that ReadFile() writes to your buffer is not null-terminated, but you are appending a null-terminated char* pointer to your output std::string. You need to pay attention to the actual size that ReadFile() reports to you, especially since it doesn't require a null terminator, and it may be smaller than what you asked for, eg:

    std::array<char, 512> buffer;
    std::string adbConnectOutput;
    DWORD dwNumRead;
    
    while (ReadFile(stdoutReadHandle, buffer.data(), buffer.size(), &dwNumRead, NULL))
        adbConnectOutput.append(buffer.data(), dwNumRead);