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

373 Views Asked by At

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
}
1

There are 1 best solutions below

5
Remy Lebeau On BEST ANSWER

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);