On Mon, 2008-10-20 at 15:02 -0400, Carolyn Beeton wrote:
> I've attached patches which implement the pipes described in
> http://track.sipfoundry.org/browse/XECS-1423 for stdout and stderr.
> The first patch is the OS component, which intercepts the output in
> such a way that the application can (optionally) get it while it waits
> for the child to terminate. The second patch is the application level
> code to ask for the terminal output, and log it. The result is a much
> cleaner look when running from a terminal. All output to stdout is
> logged (along with the service name) in sipxsupervisor.log at level
> PRI_INFO, and stderr output with PRI_ERR.
Some questions:
1. Is the capturing of stdout/stderr mandatory? Or can I use OsProcess
in a way that is like before? Does the default behavior change?
2. The comment on getOutput isn't so clear. Perhaps this is an
improvement:
/// Wait until the process produces output on stdout or stderr, or
/// exits. Returns -1 if process exits. If output is obtained,
/// sets 'message' to the output and 'fd' to STDOUT_FILENO or STDERR_FILENO
/// depending on which fd output was obtained from, then returns >0.
/// 'message' values may be broken at arbitrary points.
virtual int getOutput(int& fd, UtlString& message) = 0;
3. There are a number of places where printf's are commented out. These
should be converted into a suitable #ifdef.
4. The placement of the null is incorrect here:
+ // read until all input has been read
+ rc = read(realFd, buf, sizeof(buf)-1);
+ buf[sizeof(buf)-1]=0;
as it will be incorrect if rc < sizeof(buf)-1.
5. But I think we can omit placing a null at all using code like:
printf("read %d bytes more: %.*s\n", rc, rc, buf);
message.append(buf, rc);
6. I think the main loop of OsProcessLinux::getOutput could be
simplified to something like:
// read until all input has been read
message.remove(0);
do
{
rc = read(realFd, buf, sizeof(buf)-1);
if (rc > 0)
{
// printf("read %d bytes: %.*s\n", rc, rc, buf);
message.append(buf, rc);
}
} while (rc == sizeof(buf)-1);
// If output was obtained, return its length.
// But if no output was obtained, return the 0 or -1 that
// read() returned.
if (message.length() > 0)
{
rc = message.length();
}
7. There might be some ugly behavior if the process closes stdout or
stderr significantly before it exits. From the man pages, I would
expect select() to always return ready-to-read on the other end of the
closed pipe, and read() on it to return 0 (EOF), so getOutput would
always return immediately.
Dale
_______________________________________________
sipx-dev mailing list
[email protected]
List Archive: http://list.sipfoundry.org/archive/sipx-dev
Unsubscribe: http://list.sipfoundry.org/mailman/listinfo/sipx-dev
