This is an archive of the discontinued LLVM Phabricator instance.

Fix TestProcessIO.py when run against a remote target
ClosedPublic

Authored by vharron on Feb 10 2015, 5:25 PM.

Details

Reviewers
ovyalov
clayborg
Summary

Fixed test case to copy redirected stdout/stderr files from remote
target to host

llgs wasn't bothering to put the pty master file handle in the right
place if stdout/stderr were redirected to a file. It is still needed
for stdin.

Fixed some erroneous log messages

Diff Detail

Event Timeline

vharron updated this revision to Diff 19723.Feb 10 2015, 5:25 PM
vharron retitled this revision from to Fix TestProcessIO.py when run against a remote target.
vharron updated this object.
vharron edited the test plan for this revision. (Show Details)
vharron added reviewers: ovyalov, clayborg.
vharron added a subscriber: Unknown Object (MLST).
ovyalov accepted this revision.Feb 10 2015, 6:04 PM
ovyalov edited edge metadata.
ovyalov added inline comments.
test/python_api/process/io/TestProcessIO.py
128–129

read_output_file_and_delete and read_error_file_and_delete look pretty identical -
we may have have common function read_file_and_delete(self, local_file) which is used by read_output_file_and_delete as read_file_and_delete(self.local_output_file) and read_error_file_and_delete as read_file_and_delete(self.local_error_file)

This revision is now accepted and ready to land.Feb 10 2015, 6:04 PM
vharron updated this revision to Diff 19771.Feb 11 2015, 11:04 AM
vharron edited edge metadata.

refactored common code out of read_output/error_file_and_delete

clayborg requested changes to this revision.Feb 11 2015, 1:37 PM
clayborg edited edge metadata.

See my inlined code comments.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
503 ↗(On Diff #19771)

Should this be:

if (launch_info.GetFileActionForFD (STDOUT_FILENO) == nullptr &&
    launch_info.GetFileActionForFD (STDERR_FILENO) == nullptr)
525–543 ↗(On Diff #19771)

Remove this in favor of just inlining the following code?

if (launch_info.GetFileActionForFD (STDOUT_FILENO) == nullptr &&
    launch_info.GetFileActionForFD (STDERR_FILENO) == nullptr)

thoughts? You could add new functions to ProcessLaunch if you want to make it:

if (!launch_info.HasFileActionFor(STDOUT_FILENO) && !launch_info.HasFileAction(STDERR_FILENO))

Or if you added a var arg version:

if (!launch_info.HasFileActionForAnyOf(STDOUT_FILENO, STDERR_FILENO))
572 ↗(On Diff #19771)

Inline accessors to m_process_launch_info as state above?

1897 ↗(On Diff #19771)

Get rid of this empty line diff?

This revision now requires changes to proceed.Feb 11 2015, 1:37 PM
vharron updated this revision to Diff 19856.Feb 12 2015, 1:49 PM
vharron edited edge metadata.

Removed functions for checking stdio over gdb-remote

Single line diff left intentionally to separate functions by a blank line (convention/standard)

clayborg accepted this revision.Feb 12 2015, 2:07 PM
clayborg edited edge metadata.

Looks good

This revision is now accepted and ready to land.Feb 12 2015, 2:07 PM
vharron updated this revision to Diff 19909.Feb 13 2015, 11:14 AM
vharron edited edge metadata.

rebased against ToT

vharron closed this revision.Feb 13 2015, 11:17 AM

Committed revision 229141.