Re: CreateProcess CreatePipe - gather Stderr output not working

On Tue, 23 Dec 2008 20:08:02 -0800 (PST), Bob <jeep@xxxxxxxxx> wrote:

Im trying to create a process and send the output to a pipe which I
read after the
process runs so I can see whats wrong. In this case Im trying to call
it with pscp
(which is why I hardcoded the error Im looking for btw). Ive tried
googling this and
Ive seen many variations of this code, but so far I cant get it to
work the way I was
expecting it to.

In my debug output, ReadFile Avail <0> is as far as it gets and then
'hangs'.
I know its hanging because when I do it in the command line It also
hangs with the
string Im looking for, "Server refused our key", but I cant see why
the following code
isnt getting my Stderr output as I want. And yes, Im a newbie still.

Id appreciate any help on getting this to work and bulletproofing it
more also.
Thanks

bool doSystem(LPSTR cmd)
****
That should be LPTSTR. LPSTR is dead except in rare and exotic uses, which this is not an
example of
****
{
HANDLE read_stderr, wr_stderr;
****
Are you sure it is going to stderr and not stdout?
****

STARTUPINFO si;
ZeroMemory(&si, sizeof(si));
si.cb = sizeof(si); // Must set the size of structure

PROCESS_INFORMATION pi;
SECURITY_ATTRIBUTES sa;
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
sa.bInheritHandle = FALSE;
****
If you don't create inheritable handles, you can't use them. So why are you specifically
requesting non-inheritable handles?
SECURITY_ATTRIBUTES sa = { sizeof(SECURITY_ATTRIBUTES), NULL, TRUE};
is the correct declaration.
****
sa.lpSecurityDescriptor = NULL;

if (!CreatePipe(&read_stderr, &wr_stderr, &sa, 0)) //create
stderr pipe
****
No, you cannot use this pipe because you have not created an inheritable pipe. So it
won't work.

And are you ABSOLUTELY certain the child process is writing its message to stderr and not
stdout? Note also, are you ABSOLUTELY certain the child process doesn't use stdout
exclusively and just close stderr by default?
****
{
dmsg(TERR, "CreatePipe %d", GetLastError());
return false;
}

si.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW;
si.wShowWindow = SW_HIDE;
si.hStdError = wr_stderr;

BOOL ret = CreateProcess(NULL, cmd, NULL, NULL, FALSE,
****
In addition to creating an inheritable handle, you have to tell CreateProcess to inherit
the inheritable handles. So even if you fix the above errors, you still will not get
stderr, because you used FALSE instead of TRUE as the inherit parameter.
****
CREATE_NEW_CONSOLE, //CREATE_NO_WINDOW,
NULL, NULL, &si, &pi);
if (!ret)
{
dmsg(TERR, "CreateProcess FAILED");
return false;
}
//WaitForSingleObject(pi.hProcess, INFINITE);
****
Of course, the above is completely inappropriate, because no progress can be made until
after the process has terminated
****

dmsg(TINFO, "Process Started");

char buf[MAX_BUF];
DWORD bread, avail;

if (!PeekNamedPipe(read_stderr, buf, MAX_BUF - 1, &bread, &avail,
NULL))
{
dmsg(TERR, "PeekNamedPipe stderr");
****
Why do you think it is appropriate that PeekNamedPipe will instantly say there is
something there? The other process may not even be running yet!!! So this code is
inappropriate. It presumes that the process has sent everything before this code is
executed, which means the process has to come up, do whatever it needs to do, and write
its error message out in a few hundred nanoseconds, which seems unlikely. In addition, it
might take several ReadFile operations to read the output; there's no guarantee that a
single one will read everything that the process wants to write.

Also, I'm not sure why you are using a 'char' buffer of fixed size; you could consider
using a CStringA and appending to it, but I'll leave that as an Exercise For The Reader.
****
}
else
{
dmsg(TINFO, "ReadFile Avail <%d>", avail);
ReadFile(read_stderr, buf, 1023, &bread, NULL);
****
Why 1023? WHat does 1023 have to do with MAX_BUF?
****
dmsg(TINFO, "bread <%d>", bread);
dmsg(TINFO, "BUF: <%s>", buf);
if (bread > 5)
{
if (!strcmp(buf, "Server refused our key"))
{
dmsg(TERR, "Server refused our key");
}
}
}
*****
Eliminate everything above this point, up to and including the PeekNamedPipe,

CloseHandle(wr_stderr);
BYTE * b = buff;
bread = 0;
DWORD len = MAX_BUF - 1;
while(true)
{ /* read loop */
if(!ReadFile(read_stderr, b, len, &bread, NULL))
{ /* error */
DWORD err = ::GetLastError();
if(err == ERROR_BROKEN_PIPE)
break; // process terminated
dmsg(TERR, _T("ReadFile failed %d"), err);
break;
} /* error */
b += bread;
*b = '\0';
len -= bread;
if(len == 0)
break;
continue;
} /* read loop */
*****
CloseHandle(wr_stderr);
*****
Note this was done earlier. You have to close the child process side of the pipe or you
won't get the broken pipe error. Now you can close your side of the pipe (which you
forgot to do) and the thread handle also (which you forgot to do)

CloseHandle(read_stderr);
CloseHandle(pi.hThread);

joe
*****
CloseHandle(pi.hProcess);

return true;
}
Joseph M. Newcomer [MVP]
email: newcomer@xxxxxxxxxxxx
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
.

Relevant Pages