Re: [PATCH 7/7] gdbserver: remove support for ARM/WinCE

[Kevin Buettner <kevinb@redhat.com>]

Hi Simon,

Just one nit, maybe...

On Thu, 14 May 2020 15:05:37 -0400
Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> wrote:

> diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
> index 4eb63b7ca25a..d671691a575d 100644
> --- a/gdbserver/win32-low.cc
> +++ b/gdbserver/win32-low.cc
> @@ -1414,69 +1414,39 @@ get_child_debug_event (DWORD *continue_status,
>        goto gotevent;
>      }
>  
> -#ifndef _WIN32_WCE
>    attaching = 0;
> -#else
> -  if (attaching)
> -    {
> -      /* WinCE doesn't set an initial breakpoint automatically.  To
> -	 stop the inferior, we flush all currently pending debug
> -	 events -- the thread list and the dll list are always
> -	 reported immediatelly without delay, then, we suspend all
> -	 threads and pretend we saw a trap at the current PC of the
> -	 main thread.
> -
> -	 Contrary to desktop Windows, Windows CE *does* report the dll
> -	 names on LOAD_DLL_DEBUG_EVENTs resulting from a
> -	 DebugActiveProcess call.  This limits the way we can detect
> -	 if all the dlls have already been reported.  If we get a real
> -	 debug event before leaving attaching, the worst that will
> -	 happen is the user will see a spurious breakpoint.  */
> -
> -      current_event.dwDebugEventCode = 0;
> -      if (!wait_for_debug_event (&current_event, 0))
> -	{
> -	  OUTMSG2(("no attach events left\n"));
> -	  fake_breakpoint_event ();
> -	  attaching = 0;
> -	}
> -      else
> -	OUTMSG2(("got attach event\n"));
> -    }
> -  else
> -#endif
> -    {
> -      gdb::optional<pending_stop> stop = fetch_pending_stop (debug_threads);
> -      if (stop.has_value ())
> -	{
> -	  *ourstatus = stop->status;
> -	  current_event = stop->event;
> -	  ptid = debug_event_ptid (&current_event);
> -	  current_thread = find_thread_ptid (ptid);
> -	  return 1;
> -	}
> +  {

I think this brace and the matching one later on can be removed, with
a corresponding reduction in indent level for the enclosed block.

> +    gdb::optional<pending_stop> stop = fetch_pending_stop (debug_threads);
> +    if (stop.has_value ())
> +      {
> +	*ourstatus = stop->status;
> +	current_event = stop->event;
> +	ptid = debug_event_ptid (&current_event);
> +	current_thread = find_thread_ptid (ptid);
> +	return 1;
> +      }
>  
> -      /* Keep the wait time low enough for comfortable remote
> -	 interruption, but high enough so gdbserver doesn't become a
> -	 bottleneck.  */
> -      if (!wait_for_debug_event (&current_event, 250))
> -        {
> -	  DWORD e  = GetLastError();
> +    /* Keep the wait time low enough for comfortable remote
> +       interruption, but high enough so gdbserver doesn't become a
> +       bottleneck.  */
> +    if (!wait_for_debug_event (&current_event, 250))
> +      {
> +	DWORD e  = GetLastError();
>  
> -	  if (e == ERROR_PIPE_NOT_CONNECTED)
> -	    {
> -	      /* This will happen if the loader fails to succesfully
> -		 load the application, e.g., if the main executable
> -		 tries to pull in a non-existing export from a
> -		 DLL.  */
> -	      ourstatus->kind = TARGET_WAITKIND_EXITED;
> -	      ourstatus->value.integer = 1;
> -	      return 1;
> -	    }
> +	if (e == ERROR_PIPE_NOT_CONNECTED)
> +	  {
> +	    /* This will happen if the loader fails to succesfully
> +	       load the application, e.g., if the main executable
> +	       tries to pull in a non-existing export from a
> +	       DLL.  */
> +	    ourstatus->kind = TARGET_WAITKIND_EXITED;
> +	    ourstatus->value.integer = 1;
> +	    return 1;
> +	  }
>  
> -	  return 0;
> -        }
> -    }
> +	return 0;
> +      }
> +  }

I think that's the matching brace, above.

Kevin