Christopher Faylor wrote:

>> -/* Find a thread record given a thread id.
>> -   If get_context then also retrieve the context for this
>> -   thread. */
>> +/* Find a thread record given a thread id passed in ID.  If
>> +   GET_CONTEXT is not 0, then also retrieve the context for this
>> +   thread.  If GET_CONTEXT is negative, then don't suspend the
>> +   thread.  */
> 
> I don't see any reason to capitalize get_context in the comment.
> 

That's what the GNU coding standards prefer:

[5.2 Commenting Your Work]
"The comment on a function is much clearer if you use the argument
names to speak about the argument values.  The variable name
itself should be lower case, but write it in upper case when
you are speaking about the value rather than the variable
itself.  Thus, “the node number NODE_NUM” rather than “an inode”."

In fact, I shouldn't be capitalizing ID.  Fixed that.

>> @@ -294,8 +304,7 @@ win32_add_thread (DWORD id, HANDLE h)
>>       th->context.Dr1 = dr[1];
>>       th->context.Dr2 = dr[2];
>>       th->context.Dr3 = dr[3];
>> -      /* th->context.Dr6 = dr[6];
>> -      FIXME: should we set dr6 also ?? */
>> +      th->context.Dr6 = 0xffff0ff0;
> 
> This, and similar cases, needs to use a #define with an explanatory comment.
> 
> With the above minor changes, this looks ok.
> 

Added a #define and checked in (see attached).

> I have to ask, however, if the SuspendThread's are even needed at all.
> When I was writing this code, I wasn't entirely sure that gdb needed to
> do anything like this but I erred on the side of caution.  So, I'm
> wondering if things would still work ok if the
> SuspendThread/ResumeThread stuff was gone.
> 

I wonder that too.  The docs say that we must not retrieve a thread
context while the thread is running.  When the inferior is stopped due
to a debug event the whole process is frozen, so in that case it should
safe.  I can give it a try later on, but it may take me a while to
confirm if it really is safe, especially because I currently only
have access to Windows XP SP2 and WinCE  (from the archives, I
get the impression Win2000 is pickier.)

OTOH, we'll still need to suspend threads if we want
to implement support for debugging one thread while the
others are running, or to step just one thread while
the others are stopped, or if we want to support stopping the
inferior without relying on a console available to handle
GenerateConsoleCtrlEvent; but these are different stories.

-- 
Pedro Alves