Looks fine, thanks for the fix. On Wed, Oct 22, 2014 at 9:30 AM, Abid, Hafiz <[email protected]> wrote:
> Hi Zachary, > Some 'goto' jumps over local variable with initialization in > ConnectionGenericFileWindows.cpp. > I think this makes the code ill-formed as per C++ standard 6.7.3. I have > attached a simple > fix for it. Please let me know if you see a problem with it. Otherwise I > will apply it later today. > > Thanks, > Abid > > > -----Original Message----- > > From: [email protected] [mailto:lldb-commits- > > [email protected]] On Behalf Of Zachary Turner > > Sent: 06 October 2014 22:23 > > To: [email protected] > > Subject: [Lldb-commits] [lldb] r219145 - Create a ConnectionGenericFile > class > > for Windows. > > > > Author: zturner > > Date: Mon Oct 6 16:23:09 2014 > > New Revision: 219145 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=219145&view=rev > > Log: > > Create a ConnectionGenericFile class for Windows. > > > > This is the first step in getting ConnectionFileDescriptor ported to > Windows. > > It implements a connection against a disk file for windows. This supports > > connection strings of the form file://PATH which are currently supported > > only on posix platforms in ConnectionFileDescriptor. > > > > Reviewed by: Greg Clayton > > Differential Revision: http://reviews.llvm.org/D5608 > > > > Added: > > lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h > > lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp > > Modified: > > lldb/trunk/include/lldb/Core/Connection.h > > lldb/trunk/include/lldb/lldb-types.h > > lldb/trunk/source/API/SBCommunication.cpp > > lldb/trunk/source/Core/Connection.cpp > > lldb/trunk/source/Host/CMakeLists.txt > > > > Modified: lldb/trunk/include/lldb/Core/Connection.h > > URL: http://llvm.org/viewvc/llvm- > > project/lldb/trunk/include/lldb/Core/Connection.h?rev=219145&r1=219144 > > &r2=219145&view=diff > > ================================================================ > > ============== > > --- lldb/trunk/include/lldb/Core/Connection.h (original) > > +++ lldb/trunk/include/lldb/Core/Connection.h Mon Oct 6 16:23:09 2014 > > @@ -46,6 +46,8 @@ public: > > virtual > > ~Connection (); > > > > + static Connection *CreateDefaultConnection(const char *url); > > + > > //------------------------------------------------------------------ > > /// Connect using the connect string \a url. > > /// > > > > Added: > > lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h > > URL: http://llvm.org/viewvc/llvm- > > project/lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindo > > ws.h?rev=219145&view=auto > > ================================================================ > > ============== > > --- lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h > > (added) > > +++ > > lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h > > +++ Mon Oct 6 16:23:09 2014 > > @@ -0,0 +1,64 @@ > > +//===-- ConnectionGenericFileWindows.h --------------------------*- C++ > > +-*-===// // > > +// The LLVM Compiler Infrastructure > > +// > > +// This file is distributed under the University of Illinois Open > > +Source // License. See LICENSE.TXT for details. > > +// > > +//===------------------------------------------------------------------ > > +----===// > > + > > +#ifndef liblldb_Host_windows_ConnectionGenericFileWindows_h_ > > +#define liblldb_Host_windows_ConnectionGenericFileWindows_h_ > > + > > +#include "lldb/Core/Connection.h" > > +#include "lldb/Host/windows/windows.h" > > +#include "lldb/lldb-types.h" > > + > > +namespace lldb_private > > +{ > > + > > +class Error; > > + > > +class ConnectionGenericFile : public lldb_private::Connection { > > + public: > > + ConnectionGenericFile(); > > + > > + ConnectionGenericFile(lldb::file_t file, bool owns_file); > > + > > + virtual ~ConnectionGenericFile(); > > + > > + virtual bool IsConnected() const; > > + > > + virtual lldb::ConnectionStatus Connect(const char *s, Error > > + *error_ptr); > > + > > + virtual lldb::ConnectionStatus Disconnect(Error *error_ptr); > > + > > + virtual size_t Read(void *dst, size_t dst_len, uint32_t > > + timeout_usec, lldb::ConnectionStatus &status, Error *error_ptr); > > + > > + virtual size_t Write(const void *src, size_t src_len, > > + lldb::ConnectionStatus &status, Error *error_ptr); > > + > > + bool InterruptRead(); > > + > > + protected: > > + OVERLAPPED m_overlapped; > > + HANDLE m_file; > > + HANDLE m_event_handles[2]; > > + bool m_owns_file; > > + LARGE_INTEGER m_file_position; > > + > > + enum > > + { > > + kBytesAvailableEvent, > > + kInterruptEvent > > + }; > > + > > + private: > > + void InitializeEventHandles(); > > + void IncrementFilePointer(DWORD amount); > > + > > + DISALLOW_COPY_AND_ASSIGN(ConnectionGenericFile); > > +}; > > +} > > + > > +#endif > > > > Modified: lldb/trunk/include/lldb/lldb-types.h > > URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb- > > types.h?rev=219145&r1=219144&r2=219145&view=diff > > ================================================================ > > ============== > > --- lldb/trunk/include/lldb/lldb-types.h (original) > > +++ lldb/trunk/include/lldb/lldb-types.h Mon Oct 6 16:23:09 2014 > > @@ -51,6 +51,9 @@ namespace lldb > > typedef void* rwlock_t; > > typedef void* process_t; // Process > type is HANDLE > > typedef void* thread_t; // Host > thread type > > + typedef void* file_t; // Host > file type > > + typedef void* pipe_t; // Host > pipe type > > + typedef unsigned int __w64 socket_t; // Host > socket type > > typedef uint32_t thread_key_t; > > typedef void* thread_arg_t; // Host > thread argument > > type > > typedef unsigned thread_result_t; // Host > thread result type > > @@ -71,6 +74,9 @@ namespace lldb > > typedef pthread_rwlock_t rwlock_t; > > typedef uint64_t process_t; // Process > type is just a pid. > > typedef pthread_t thread_t; // Host > thread type > > + typedef int file_t; // Host > file type > > + typedef int pipe_t; // Host > pipe type > > + typedef int socket_t; // Host > socket type > > typedef pthread_key_t thread_key_t; > > typedef void * thread_arg_t; // Host > thread argument > > type > > typedef void * thread_result_t; // Host > thread result type > > > > Modified: lldb/trunk/source/API/SBCommunication.cpp > > URL: http://llvm.org/viewvc/llvm- > > project/lldb/trunk/source/API/SBCommunication.cpp?rev=219145&r1=21914 > > 4&r2=219145&view=diff > > ================================================================ > > ============== > > --- lldb/trunk/source/API/SBCommunication.cpp (original) > > +++ lldb/trunk/source/API/SBCommunication.cpp Mon Oct 6 16:23:09 2014 > > @@ -71,7 +71,7 @@ SBCommunication::Connect (const char *ur > > if (m_opaque) > > { > > if (!m_opaque->HasConnection ()) > > - m_opaque->SetConnection (new ConnectionFileDescriptor()); > > + > > + m_opaque->SetConnection(Connection::CreateDefaultConnection(url)); > > return m_opaque->Connect (url, NULL); > > } > > return eConnectionStatusNoConnection; > > > > Modified: lldb/trunk/source/Core/Connection.cpp > > URL: http://llvm.org/viewvc/llvm- > > project/lldb/trunk/source/Core/Connection.cpp?rev=219145&r1=219144&r2 > > =219145&view=diff > > ================================================================ > > ============== > > --- lldb/trunk/source/Core/Connection.cpp (original) > > +++ lldb/trunk/source/Core/Connection.cpp Mon Oct 6 16:23:09 2014 > > @@ -13,6 +13,12 @@ > > // Project includes > > #include "lldb/Core/Connection.h" > > > > +#if defined(_WIN32) > > +#include "lldb/Host/windows/ConnectionGenericFileWindows.h" > > +#endif > > + > > +#include "lldb/Host/ConnectionFileDescriptor.h" > > + > > using namespace lldb_private; > > > > Connection::Connection () > > @@ -22,3 +28,13 @@ Connection::Connection () Connection::~Connection () > > { } > > + > > +Connection * > > +Connection::CreateDefaultConnection(const char *url) { #if > > +defined(_WIN32) > > + if (strstr(url, "file://") == url) > > + return new ConnectionGenericFile(); #endif > > + return new ConnectionFileDescriptor(); } > > > > Modified: lldb/trunk/source/Host/CMakeLists.txt > > URL: http://llvm.org/viewvc/llvm- > > project/lldb/trunk/source/Host/CMakeLists.txt?rev=219145&r1=219144&r2= > > 219145&view=diff > > ================================================================ > > ============== > > --- lldb/trunk/source/Host/CMakeLists.txt (original) > > +++ lldb/trunk/source/Host/CMakeLists.txt Mon Oct 6 16:23:09 2014 > > @@ -40,6 +40,7 @@ add_host_subdirectory(posix if > > (CMAKE_SYSTEM_NAME MATCHES "Windows") > > add_host_subdirectory(windows > > windows/Condition.cpp > > + windows/ConnectionGenericFileWindows.cpp > > windows/EditLineWin.cpp > > windows/FileSystem.cpp > > windows/Host.cpp > > > > Added: > > lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp > > URL: http://llvm.org/viewvc/llvm- > > project/lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cp > > p?rev=219145&view=auto > > ================================================================ > > ============== > > --- lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp > > (added) > > +++ lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp > > Mon > > +++ Oct 6 16:23:09 2014 > > @@ -0,0 +1,337 @@ > > +//===-- ConnectionGenericFileWindows.cpp ------------------------*- C++ > > +-*-===// // > > +// The LLVM Compiler Infrastructure > > +// > > +// This file is distributed under the University of Illinois Open > > +Source // License. See LICENSE.TXT for details. > > +// > > +//===------------------------------------------------------------------ > > +----===// > > + > > +#include "lldb/Core/Error.h" > > +#include "lldb/Core/Log.h" > > +#include "lldb/Host/TimeValue.h" > > +#include "lldb/Host/windows/ConnectionGenericFileWindows.h" > > + > > +#include "llvm/ADT/STLExtras.h" > > +#include "llvm/ADT/StringRef.h" > > + > > +using namespace lldb; > > +using namespace lldb_private; > > + > > +namespace > > +{ > > +// This is a simple helper class to package up the information needed > > +to return from a Read/Write // operation function. Since there is alot > > +of code to be run before exit regardless of whether the // operation > > +succeeded or failed, combined with many possible return paths, this is > the > > cleanest // way to represent it. > > +class ReturnInfo > > +{ > > + public: > > + void > > + Set(size_t bytes, ConnectionStatus status, DWORD error_code) > > + { > > + m_error.SetError(error_code, eErrorTypeWin32); > > + m_bytes = bytes; > > + m_status = status; > > + } > > + > > + void > > + Set(size_t bytes, ConnectionStatus status, llvm::StringRef > error_msg) > > + { > > + m_error.SetErrorString(error_msg.data()); > > + m_bytes = bytes; > > + m_status = status; > > + } > > + > > + size_t > > + GetBytes() const > > + { > > + return m_bytes; > > + } > > + ConnectionStatus > > + GetStatus() const > > + { > > + return m_status; > > + } > > + const Error & > > + GetError() const > > + { > > + return m_error; > > + } > > + > > + private: > > + Error m_error; > > + size_t m_bytes; > > + ConnectionStatus m_status; > > +}; > > +} > > + > > +ConnectionGenericFile::ConnectionGenericFile() > > + : m_file(INVALID_HANDLE_VALUE) > > + , m_owns_file(false) > > +{ > > + ::ZeroMemory(&m_overlapped, sizeof(m_overlapped)); > > + ::ZeroMemory(&m_file_position, sizeof(m_file_position)); > > + InitializeEventHandles(); > > +} > > + > > +ConnectionGenericFile::ConnectionGenericFile(lldb::file_t file, bool > > owns_file) > > + : m_file(file) > > + , m_owns_file(owns_file) > > +{ > > + ::ZeroMemory(&m_overlapped, sizeof(m_overlapped)); > > + ::ZeroMemory(&m_file_position, sizeof(m_file_position)); > > + InitializeEventHandles(); > > +} > > + > > +ConnectionGenericFile::~ConnectionGenericFile() > > +{ > > + if (m_owns_file && IsConnected()) > > + ::CloseHandle(m_file); > > + > > + ::CloseHandle(m_event_handles[kBytesAvailableEvent]); > > + ::CloseHandle(m_event_handles[kInterruptEvent]); > > +} > > + > > +void > > +ConnectionGenericFile::InitializeEventHandles() > > +{ > > + m_event_handles[kInterruptEvent] = CreateEvent(NULL, FALSE, FALSE, > > +NULL); > > + > > + // Note, we should use a manual reset event for the hEvent argument > of > > the OVERLAPPED. This > > + // is because both WaitForMultipleObjects and GetOverlappedResult > (if > > you set the bWait > > + // argument to TRUE) will wait for the event to be signalled. If > we use an > > auto-reset event, > > + // WaitForMultipleObjects will reset the event, return > successfully, and > > then > > + // GetOverlappedResult will block since the event is no longer > signalled. > > + m_event_handles[kBytesAvailableEvent] = ::CreateEvent(NULL, TRUE, > > +FALSE, NULL); } > > + > > +bool > > +ConnectionGenericFile::IsConnected() const { > > + return m_file && (m_file != INVALID_HANDLE_VALUE); } > > + > > +lldb::ConnectionStatus > > +ConnectionGenericFile::Connect(const char *s, Error *error_ptr) { > > + Log > > *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); > > + if (log) > > + log->Printf("%p ConnectionGenericFile::Connect (url = '%s')", > > +static_cast<void *>(this), s); > > + > > + if (strstr(s, "file://") != s) > > + { > > + if (error_ptr) > > + error_ptr->SetErrorStringWithFormat("unsupported connection > URL: > > '%s'", s); > > + return eConnectionStatusError; > > + } > > + > > + if (IsConnected()) > > + { > > + ConnectionStatus status = Disconnect(error_ptr); > > + if (status != eConnectionStatusSuccess) > > + return status; > > + } > > + > > + // file://PATH > > + const char *path = s + strlen("file://"); > > + // Open the file for overlapped access. If it does not exist, > create it. We > > open it overlapped > > + // so that we can issue asynchronous reads and then use > > WaitForMultipleObjects to allow the read > > + // to be interrupted by an event object. > > + m_file = ::CreateFile(path, GENERIC_READ | GENERIC_WRITE, > > FILE_SHARE_READ, NULL, OPEN_ALWAYS, FILE_FLAG_OVERLAPPED, NULL); > > + if (m_file == INVALID_HANDLE_VALUE) > > + { > > + if (error_ptr) > > + error_ptr->SetError(::GetLastError(), eErrorTypeWin32); > > + return eConnectionStatusError; > > + } > > + > > + m_owns_file = true; > > + return eConnectionStatusSuccess; > > +} > > + > > +lldb::ConnectionStatus > > +ConnectionGenericFile::Disconnect(Error *error_ptr) { > > + Log > > *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); > > + if (log) > > + log->Printf("%p ConnectionGenericFile::Disconnect (url = > > +'%s')", static_cast<void *>(this), s); > > + > > + if (!IsConnected()) > > + return eConnectionStatusSuccess; > > + > > + // Reset the handle so that after we unblock any pending reads, > > subsequent calls to Read() will > > + // see a disconnected state. > > + HANDLE old_file = m_file; > > + m_file = INVALID_HANDLE_VALUE; > > + > > + // Set the disconnect event so that any blocking reads unblock, then > > cancel any pending IO operations. > > + ::CancelIoEx(old_file, &m_overlapped); > > + > > + // Close the file handle if we owned it, but don't close the event > handles. > > We could always > > + // reconnect with the same Connection instance. > > + if (m_owns_file) > > + ::CloseHandle(old_file); > > + > > + ::ZeroMemory(&m_file_position, sizeof(m_file_position)); > > + m_owns_file = false; > > + return eConnectionStatusSuccess; > > +} > > + > > +size_t > > +ConnectionGenericFile::Read(void *dst, size_t dst_len, uint32_t > > +timeout_usec, lldb::ConnectionStatus &status, Error *error_ptr) { > > + ReturnInfo return_info; > > + > > + if (error_ptr) > > + error_ptr->Clear(); > > + > > + if (!IsConnected()) > > + { > > + return_info.Set(0, eConnectionStatusNoConnection, > > ERROR_INVALID_HANDLE); > > + goto finish; > > + } > > + > > + m_overlapped.hEvent = m_event_handles[kBytesAvailableEvent]; > > + > > + BOOL result = ::ReadFile(m_file, dst, dst_len, NULL, &m_overlapped); > > + if (result || ::GetLastError() == ERROR_IO_PENDING) > > + { > > + if (!result) > > + { > > + // The expected return path. The operation is pending. > Wait for > > the operation to complete > > + // or be interrupted. > > + TimeValue time_value; > > + time_value.OffsetWithMicroSeconds(timeout_usec); > > + DWORD milliseconds = time_value.milliseconds(); > > + result = > > ::WaitForMultipleObjects(llvm::array_lengthof(m_event_handles), > > m_event_handles, FALSE, milliseconds); > > + // All of the events are manual reset events, so make sure > we reset > > them to non-signalled. > > + switch (result) > > + { > > + case WAIT_OBJECT_0 + kBytesAvailableEvent: > > + break; > > + case WAIT_OBJECT_0 + kInterruptEvent: > > + return_info.Set(0, eConnectionStatusInterrupted, 0); > > + goto finish; > > + case WAIT_TIMEOUT: > > + return_info.Set(0, eConnectionStatusTimedOut, 0); > > + goto finish; > > + case WAIT_FAILED: > > + return_info.Set(0, eConnectionStatusError, > ::GetLastError()); > > + goto finish; > > + } > > + } > > + // The data is ready. Figure out how much was read and return; > > + DWORD bytes_read = 0; > > + if (!::GetOverlappedResult(m_file, &m_overlapped, &bytes_read, > > FALSE)) > > + { > > + DWORD result_error = ::GetLastError(); > > + // ERROR_OPERATION_ABORTED occurs when someone calls > > Disconnect() during a blocking read. > > + // This triggers a call to CancelIoEx, which causes the > operation to > > complete and the > > + // result to be ERROR_OPERATION_ABORTED. > > + if (result_error == ERROR_HANDLE_EOF || result_error == > > ERROR_OPERATION_ABORTED) > > + return_info.Set(bytes_read, eConnectionStatusEndOfFile, > 0); > > + else > > + return_info.Set(bytes_read, eConnectionStatusError, > > result_error); > > + } > > + else if (bytes_read == 0) > > + return_info.Set(bytes_read, eConnectionStatusEndOfFile, 0); > > + else > > + return_info.Set(bytes_read, eConnectionStatusSuccess, 0); > > + > > + goto finish; > > + } > > + > > + // An unknown error occured. Fail out. > > + return_info.Set(0, eConnectionStatusError, ::GetLastError()); > > + goto finish; > > + > > +finish: > > + status = return_info.GetStatus(); > > + if (error_ptr) > > + *error_ptr = return_info.GetError(); > > + > > + // kBytesAvailableEvent is a manual reset event. Make sure it gets > reset > > here so that any > > + // subsequent operations don't immediately see bytes available. > > + ResetEvent(m_event_handles[kBytesAvailableEvent]); > > + > > + IncrementFilePointer(return_info.GetBytes()); > > + Log > > *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); > > + if (log) > > + { > > + log->Printf("%" PRIxPTR " ConnectionGenericFile::Read() handle > = %" > > PRIxPTR ", dst = %" PRIxPTR ", dst_len = %" PRIu64 > > + ") => %" PRIu64 ", error = %s", > > + this, m_file, dst, static_cast<uint64_t>(dst_len), > > static_cast<uint64_t>(return_info.GetBytes()), > > + return_info.GetError().AsCString()); > > + } > > + > > + return return_info.GetBytes(); > > +} > > + > > +size_t > > +ConnectionGenericFile::Write(const void *src, size_t src_len, > > +lldb::ConnectionStatus &status, Error *error_ptr) { > > + ReturnInfo return_info; > > + > > + if (error_ptr) > > + error_ptr->Clear(); > > + > > + if (!IsConnected()) > > + { > > + return_info.Set(0, eConnectionStatusNoConnection, > > ERROR_INVALID_HANDLE); > > + goto finish; > > + } > > + > > + m_overlapped.hEvent = NULL; > > + > > + // Writes are not interruptible like reads are, so just block until > it's done. > > + BOOL result = ::WriteFile(m_file, src, src_len, NULL, > &m_overlapped); > > + if (!result && ::GetLastError() != ERROR_IO_PENDING) > > + { > > + return_info.Set(0, eConnectionStatusError, ::GetLastError()); > > + goto finish; > > + } > > + > > + DWORD bytes_written = 0; > > + if (!::GetOverlappedResult(m_file, &m_overlapped, &bytes_written, > > TRUE)) > > + { > > + return_info.Set(bytes_written, eConnectionStatusError, > > ::GetLastError()); > > + goto finish; > > + } > > + > > + return_info.Set(bytes_written, eConnectionStatusSuccess, 0); > > + goto finish; > > + > > +finish: > > + status = return_info.GetStatus(); > > + if (error_ptr) > > + *error_ptr = return_info.GetError(); > > + > > + IncrementFilePointer(return_info.GetBytes()); > > + Log > > *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); > > + if (log) > > + { > > + log->Printf("%" PRIxPTR " ConnectionGenericFile::Write() > handle = %" > > PRIxPTR ", src = %" PRIxPTR ", src_len = %" PRIu64 > > + ") => %" PRIu64 ", error = %s", > > + this, m_file, src, static_cast<uint64_t>(src_len), > > static_cast<uint64_t>(return_info.GetBytes()), > > + return_info.GetError().AsCString()); > > + } > > + return return_info.GetBytes(); > > +} > > + > > +bool > > +ConnectionGenericFile::InterruptRead() > > +{ > > + return ::SetEvent(m_event_handles[kInterruptEvent]); > > +} > > + > > +void > > +ConnectionGenericFile::IncrementFilePointer(DWORD amount) { > > + LARGE_INTEGER old_pos; > > + old_pos.HighPart = m_overlapped.OffsetHigh; > > + old_pos.LowPart = m_overlapped.Offset; > > + old_pos.QuadPart += amount; > > + m_overlapped.Offset = old_pos.LowPart; > > + m_overlapped.OffsetHigh = old_pos.HighPart; } > > \ No newline at end of file > > > > > > _______________________________________________ > > lldb-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits >
_______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
