This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/4] windows-nat: Consistently use numeric get_context parameter to thread_rec()
- From: Jon TURNEY <jon dot turney at dronecode dot org dot uk>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 10 Jun 2015 14:13:36 +0100
- Subject: Re: [PATCH 3/4] windows-nat: Consistently use numeric get_context parameter to thread_rec()
- Authentication-results: sourceware.org; auth=none
- References: <1433352592-9728-1-git-send-email-jon dot turney at dronecode dot org dot uk> <1433352592-9728-5-git-send-email-jon dot turney at dronecode dot org dot uk> <20150609191429 dot GK2855 at adacore dot com>
On 09/06/2015 20:14, Joel Brobecker wrote:
2015-06-03 Jon Turney <jon.turney@dronecode.org.uk>
* windows-nat.c : Consistently use numeric get_context parameter
to thread_rec() throughout.
I am wondering what others think of this patch.
On the one hand, it seems clearly correct. But on the other hand,
this is making me think that perhaps thread_rec's "get_context"
parameter is not very clear. What I would probably do, instead,
is split that parameter in two, one being a boolean "get_context",
and the other being a "suspend_thread" boolean.
Thanks for taking the time to review these patches.
Yes, this interface is a bit ad-hoc and far from clear.
I was looking at the code in gdbserver/win32-low.c, and the code
is similar, but not quite. There, thread_rec does not have the ability
to suspend threads. I am not sure whether whether it is an oversight
It seems be be done in win32_require_context()?
in gdbserver's code or not, but the bottom line with the current
code is that we wouldn't want to make the same change in gdbserver's
code as well. As a result and getting back to GDB's windows-nat.c,
keeping get_context as a boolean, and adding an extra one for
suspend_thread would allow more similarity between both implementations.
And given that we are hoping that, one day, we'll be able to merge
gdbserver's -low.c code with GDB's -nat code, I think similarity is
important.
It's also probably worth confirming if SuspendThread() is actually
needed at all, since the debugee is, I think, halted when
WaitForDebugEvent() returns.
That's why I would probably suggest the 2-parameters approach over
the one you've taken here. But I'd like to have other people's opinion
as well.
It seems like a good idea to me, but somebody has to do it :)