lgtm > On Feb 6, 2015, at 3:34 AM, Pavel Labath <[email protected]> wrote: > > REPOSITORY > rL LLVM > > http://reviews.llvm.org/D7440 > > Files: > lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp > lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h > lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp > lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h > lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp > > Index: lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp > =================================================================== > --- lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp > +++ lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp > @@ -2345,11 +2345,10 @@ > StopOpThread(); > sem_destroy(&m_operation_pending); > sem_destroy(&m_operation_done); > - > - // Note: ProcessPOSIX passes the m_terminal_fd file descriptor to > - // Process::SetSTDIOFileDescriptor, which in turn transfers ownership of > - // the descriptor to a ConnectionFileDescriptor object. Consequently > - // even though still has the file descriptor, we shouldn't close it here. > + if (m_terminal_fd >= 0) { > + close(m_terminal_fd); > + m_terminal_fd = -1; > + } > } > > void > Index: lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h > =================================================================== > --- lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h > +++ lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h > @@ -84,7 +84,8 @@ > /// Reads from this file descriptor yield both the standard output and > /// standard error of this debugee. Even if stderr and stdout were > /// redirected on launch it may still happen that data is available on > this > - /// descriptor (if the inferior process opens /dev/tty, for example). > + /// descriptor (if the inferior process opens /dev/tty, for example). > This descriptor is > + /// closed after a call to StopMonitor(). > /// > /// If this monitor was attached to an existing process this method > returns > /// -1. > Index: lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp > =================================================================== > --- lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp > +++ lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp > @@ -252,7 +252,16 @@ > if (!error.Success()) > return error; > > - SetSTDIOFileDescriptor(m_monitor->GetTerminalFD()); > + int terminal = m_monitor->GetTerminalFD(); > + if (terminal >= 0) { > + // The reader thread will close the file descriptor when done, so we > pass it a copy. > + int stdio = fcntl(terminal, F_DUPFD_CLOEXEC, 0); > + if (stdio == -1) { > + error.SetErrorToErrno(); > + return error; > + } > + SetSTDIOFileDescriptor(stdio); > + } > > SetID(m_monitor->GetPID()); > return error; > Index: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp > =================================================================== > --- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp > +++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp > @@ -1587,11 +1587,10 @@ > StopOpThread(); > sem_destroy(&m_operation_pending); > sem_destroy(&m_operation_done); > - > - // Note: ProcessPOSIX passes the m_terminal_fd file descriptor to > - // Process::SetSTDIOFileDescriptor, which in turn transfers ownership of > - // the descriptor to a ConnectionFileDescriptor object. Consequently > - // even though still has the file descriptor, we shouldn't close it here. > + if (m_terminal_fd >= 0) { > + close(m_terminal_fd); > + m_terminal_fd = -1; > + } > } > > // FIXME: On Linux, when a new thread is created, we receive to notifications, > Index: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h > =================================================================== > --- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h > +++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h > @@ -79,7 +79,8 @@ > /// Reads from this file descriptor yield both the standard output and > /// standard error of this debugee. Even if stderr and stdout were > /// redirected on launch it may still happen that data is available on > this > - /// descriptor (if the inferior process opens /dev/tty, for example). > + /// descriptor (if the inferior process opens /dev/tty, for example). > This descriptor is > + /// closed after a call to StopMonitor(). > /// > /// If this monitor was attached to an existing process this method > returns > /// -1. > > EMAIL PREFERENCES > http://reviews.llvm.org/settings/panel/emailpreferences/ > <D7440.19472.patch>
_______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
