From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 4E6D93840C0E for ; Thu, 14 May 2020 20:30:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4E6D93840C0E Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-243-e9qx6mPDPp6q-4sMXctfaw-1; Thu, 14 May 2020 16:30:51 -0400 X-MC-Unique: e9qx6mPDPp6q-4sMXctfaw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C198B1054F92; Thu, 14 May 2020 20:30:50 +0000 (UTC) Received: from f31-4.lan (ovpn-112-112.phx2.redhat.com [10.3.112.112]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7D4A45D9CA; Thu, 14 May 2020 20:30:50 +0000 (UTC) Date: Thu, 14 May 2020 13:30:49 -0700 From: Kevin Buettner To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: Re: [PATCH 7/7] gdbserver: remove support for ARM/WinCE Message-ID: <20200514133049.2297a58a@f31-4.lan> In-Reply-To: <20200514190537.2321826-8-simon.marchi@efficios.com> References: <20200514174359.2272960-1-simon.marchi@efficios.com> <20200514190537.2321826-8-simon.marchi@efficios.com> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-14.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 May 2020 20:31:06 -0000 Hi Simon, Just one nit, maybe... On Thu, 14 May 2020 15:05:37 -0400 Simon Marchi via Gdb-patches 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 (¤t_event, 0)) > - { > - OUTMSG2(("no attach events left\n")); > - fake_breakpoint_event (); > - attaching = 0; > - } > - else > - OUTMSG2(("got attach event\n")); > - } > - else > -#endif > - { > - gdb::optional stop = fetch_pending_stop (debug_threads); > - if (stop.has_value ()) > - { > - *ourstatus = stop->status; > - current_event = stop->event; > - ptid = debug_event_ptid (¤t_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 stop = fetch_pending_stop (debug_threads); > + if (stop.has_value ()) > + { > + *ourstatus = stop->status; > + current_event = stop->event; > + ptid = debug_event_ptid (¤t_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 (¤t_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 (¤t_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