[Lldb-commits] [lldb] r223251 - Use timeout when reading debugserver's port from a named pipe.

[Zachary Turner]

This breaks the Windows build.  For starters, Windows doesn't have
sys/select.h.  Secondly, although Windows doesn't currently use llgs, it
will in the future.  And select() on Windows does not work with Pipes.

Can we revert this change?  We have lldb/Host/Pipe.h that should be used
for pipes instead of File.  On Windows this is implemented in such a way
that it's possible to get non-blocking behavior without select, and on
posix I believe it uses select.

On Wed Dec 03 2014 at 10:25:23 AM Oleksiy Vyalov <ovyalov at google.com> wrote:

> Author: ovyalov
> Date: Wed Dec  3 12:19:16 2014
> New Revision: 223251
>
> URL: http://llvm.org/viewvc/llvm-project?rev=223251&view=rev
> Log:
> Use timeout when reading debugserver's port from a named pipe.
>
> http://reviews.llvm.org/D6490
>
>
> Modified:
>     lldb/trunk/source/Plugins/Process/gdb-remote/
> GDBRemoteCommunication.cpp
>
> Modified: lldb/trunk/source/Plugins/Process/gdb-remote/
> GDBRemoteCommunication.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/
> Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp?
> rev=223251&r1=223250&r2=223251&view=diff
> ============================================================
> ==================
> --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
> (original)
> +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
> Wed Dec  3 12:19:16 2014
> @@ -13,6 +13,7 @@
>  // C Includes
>  #include <limits.h>
>  #include <string.h>
> +#include <sys/select.h>
>  #include <sys/stat.h>
>
>  // C++ Includes
> @@ -42,6 +43,50 @@
>  using namespace lldb;
>  using namespace lldb_private;
>
> +namespace
> +{
> +
> +Error
> +ReadPortFromPipe (const char *const named_pipe_path, uint16_t& port,
> const int timeout_secs)
> +{
> +    File name_pipe_file;
> +    auto error = name_pipe_file.Open (named_pipe_path,
> File::eOpenOptionRead | File::eOpenOptionNonBlocking);
> +    if (error.Fail ())
> +        return error;
> +
> +    struct timeval tv = {timeout_secs, 0};
> +    const auto pipe_handle = name_pipe_file.GetWaitableHandle ();
> +    fd_set rfds;
> +    FD_ZERO(&rfds);
> +    FD_SET(pipe_handle, &rfds);
> +
> +    const auto retval = ::select (pipe_handle + 1, &rfds, NULL, NULL,
> &tv);
> +    if (retval == -1)
> +    {
> +        error.SetErrorToErrno ();
> +        return error;
> +    }
> +    if (retval == 0)
> +    {
> +        error.SetErrorString ("timeout exceeded");
> +        return error;
> +    }
> +
> +    char port_cstr[256];
> +    port_cstr[0] = '\0';
> +    size_t num_bytes = sizeof(port_cstr);
> +    error = name_pipe_file.Read (port_cstr, num_bytes);
> +
> +    if (error.Success ())
> +    {
> +        assert (num_bytes > 0 && port_cstr[num_bytes-1] == '\0');
> +        port = Args::StringToUInt32 (port_cstr, 0);
> +    }
> +    return error;
> +}
> +
> +}
> +
>  GDBRemoteCommunication::History::History (uint32_t size) :
>      m_packets(),
>      m_curr_idx (0),
> @@ -872,25 +917,23 @@ GDBRemoteCommunication::StartDebugserver
>          launch_info.AppendSuppressFileAction (STDIN_FILENO, true, false);
>          launch_info.AppendSuppressFileAction (STDOUT_FILENO, false,
> true);
>          launch_info.AppendSuppressFileAction (STDERR_FILENO, false,
> true);
> -
> +
>          error = Host::LaunchProcess(launch_info);
> -
> +
>          if (error.Success() && launch_info.GetProcessID() !=
> LLDB_INVALID_PROCESS_ID)
>          {
>              if (named_pipe_path[0])
>              {
> -                File name_pipe_file;
> -                error = name_pipe_file.Open(named_pipe_path,
> File::eOpenOptionRead);
> +                error = ReadPortFromPipe(named_pipe_path, out_port, 10);
>                  if (error.Success())
>                  {
> -                    char port_cstr[256];
> -                    port_cstr[0] = '\0';
> -                    size_t num_bytes = sizeof(port_cstr);
> -                    error = name_pipe_file.Read(port_cstr, num_bytes);
> -                    assert (error.Success());
> -                    assert (num_bytes > 0 && port_cstr[num_bytes-1] ==
> '\0');
> -                    out_port = Args::StringToUInt32(port_cstr, 0);
> -                    name_pipe_file.Close();
> +                    if (log)
> +                        log->Printf("GDBRemoteCommunication::%s()
> debugserver listens %u port", __FUNCTION__, out_port);
> +                }
> +                else
> +                {
> +                    if (log)
> +                        log->Printf("GDBRemoteCommunication::%s() failed
> to read a port value from named pipe %s: %s", __FUNCTION__,
> named_pipe_path, error.AsCString());
>                  }
>                  FileSystem::Unlink(named_pipe_path);
>              }
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141204/ae16541b/attachment.html>