[review] Change two windows_thread_info members to "bool"

[review] Change two windows_thread_info members to "bool"

[Tom Tromey (Code Review)]

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/409
......................................................................

Change two windows_thread_info members to "bool"

This changes a couple of fields of windows_thread_info to have type
"bool".  It also updates the comment of another field, to clarify the
possible values it can hold.

gdb/ChangeLog
2019-10-29  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (thread_rec)
	(windows_nat_target::fetch_registers): Update.
	* nat/windows-nat.h (struct windows_thread_info) <suspended>:
	Update comment.
	<debug_registers_changed, reload_context>: Now bool.

gdb/gdbserver/ChangeLog
2019-10-29  Tom Tromey  <tromey@adacore.com>

	* win32-i386-low.c (update_debug_registers)
	(i386_prepare_to_resume, i386_thread_added): Update.

Change-Id: I76e2a3a11543fde4fbd24491d96c2087de1b98b7
---
M gdb/ChangeLog
M gdb/gdbserver/ChangeLog
M gdb/gdbserver/win32-i386-low.c
M gdb/nat/windows-nat.h
M gdb/windows-nat.c
5 files changed, 23 insertions(+), 8 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d014dc8..c83fb0e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2019-10-29  Tom Tromey  <tromey@adacore.com>
 
+	* windows-nat.c (thread_rec)
+	(windows_nat_target::fetch_registers): Update.
+	* nat/windows-nat.h (struct windows_thread_info) <suspended>:
+	Update comment.
+	<debug_registers_changed, reload_context>: Now bool.
+
+2019-10-29  Tom Tromey  <tromey@adacore.com>
+
 	* windows-nat.c (windows_add_thread): Use new.
 	(windows_init_thread_list, windows_delete_thread): Use delete.
 	(get_windows_debug_event): Update.
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 5819b74..1358db3 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,10 @@
 2019-10-29  Tom Tromey  <tromey@adacore.com>
 
+	* win32-i386-low.c (update_debug_registers)
+	(i386_prepare_to_resume, i386_thread_added): Update.
+
+2019-10-29  Tom Tromey  <tromey@adacore.com>
+
 	* win32-low.c (child_add_thread): Use new.
 	(delete_thread_info): Use delete.
 
diff --git a/gdb/gdbserver/win32-i386-low.c b/gdb/gdbserver/win32-i386-low.c
index 3fc0cf1..b834b16 100644
--- a/gdb/gdbserver/win32-i386-low.c
+++ b/gdb/gdbserver/win32-i386-low.c
@@ -44,7 +44,7 @@
 
   /* The actual update is done later just before resuming the lwp,
      we just mark that the registers need updating.  */
-  th->debug_registers_changed = 1;
+  th->debug_registers_changed = true;
 }
 
 /* Update the inferior's debug register REGNUM from STATE.  */
@@ -253,14 +253,14 @@
 	 FIXME: should we set dr6 also ?? */
       th->context.Dr7 = dr->dr_control_mirror;
 
-      th->debug_registers_changed = 0;
+      th->debug_registers_changed = false;
     }
 }
 
 static void
 i386_thread_added (windows_thread_info *th)
 {
-  th->debug_registers_changed = 1;
+  th->debug_registers_changed = true;
 }
 
 static void
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 2fe2a2f..0cfc071 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -46,7 +46,9 @@
   /* Thread Information Block address.  */
   CORE_ADDR thread_local_base;
 
-  /* Non zero if SuspendThread was called on this thread.  */
+  /* This keeps track of whether SuspendThread was called on this
+     thread.  -1 means there was a failure, 1 means it was called, and
+     0 means it was not.  */
   int suspended = 0;
 
 #ifdef _WIN32_WCE
@@ -59,11 +61,11 @@
 
   /* Whether debug registers changed since we last set CONTEXT back to
      the thread.  */
-  int debug_registers_changed = 0;
+  bool debug_registers_changed = false;
 
   /* Nonzero if CONTEXT is invalidated and must be re-read from the
      inferior thread.  */
-  int reload_context = 0;
+  bool reload_context = false;
 
   /* The name of the thread, allocated by xmalloc.  */
   char *name = nullptr;
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index d18c37b..75d61cf 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -400,7 +400,7 @@
 	      }
 	    else if (get_context < 0)
 	      th->suspended = -1;
-	    th->reload_context = 1;
+	    th->reload_context = true;
 	  }
 	return th;
       }
@@ -604,7 +604,7 @@
 	      dr[7] = th->context.Dr7;
 	    }
 	}
-      th->reload_context = 0;
+      th->reload_context = false;
     }
 
   if (r < 0)

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I76e2a3a11543fde4fbd24491d96c2087de1b98b7
Gerrit-Change-Number: 409
Gerrit-PatchSet: 1
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newchange

[review] Change two windows_thread_info members to "bool"

[Luis Machado (Code Review)]

Luis Machado has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/409
......................................................................


Patch Set 1: Code-Review+1

No comments. Looks good.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I76e2a3a11543fde4fbd24491d96c2087de1b98b7
Gerrit-Change-Number: 409
Gerrit-PatchSet: 1
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Comment-Date: Thu, 31 Oct 2019 04:04:54 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[review] Change two windows_thread_info members to "bool"

[Simon Marchi (Code Review)]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/409
......................................................................


Patch Set 1: Code-Review+2


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I76e2a3a11543fde4fbd24491d96c2087de1b98b7
Gerrit-Change-Number: 409
Gerrit-PatchSet: 1
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Thu, 31 Oct 2019 18:15:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Re: [review] Change two windows_thread_info members to "bool"

[Pedro Alves]

On 10/29/19 5:57 PM, Tom Tromey (Code Review) wrote:

> --- a/gdb/nat/windows-nat.h
> +++ b/gdb/nat/windows-nat.h
> @@ -46,7 +46,9 @@
>    /* Thread Information Block address.  */
>    CORE_ADDR thread_local_base;
>  
> -  /* Non zero if SuspendThread was called on this thread.  */
> +  /* This keeps track of whether SuspendThread was called on this
> +     thread.  -1 means there was a failure, 1 means it was called, and
> +     0 means it was not.  */
>    int suspended = 0;

Re. "-1", is it really the case that -1 represents failure?

It seems like there's more to it and it should be documented differently.

Consider this call here:

static handle_exception_result
handle_exception (struct target_waitstatus *ourstatus)
{
  EXCEPTION_RECORD *rec = &current_event.u.Exception.ExceptionRecord;
  DWORD code = rec->ExceptionCode;
  handle_exception_result result = HANDLE_EXCEPTION_HANDLED;

  ourstatus->kind = TARGET_WAITKIND_STOPPED;

  /* Record the context of the current thread.  */
  thread_rec (current_event.dwThreadId, -1);
                                        ^^

I think that ends up in this code in thread_rec:

	    else if (get_context < 0)
	      th->suspended = -1;

Thanks,
Pedro Alves

[review v2] Change two windows_thread_info members to "bool"

[Tom Tromey (Code Review)]

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/409
......................................................................

Change two windows_thread_info members to "bool"

This changes a couple of fields of windows_thread_info to have type
"bool".  It also updates the comment of another field, to clarify the
possible values it can hold.

gdb/ChangeLog
2019-11-26  Tom Tromey  <tromey@adacore.com>

	* windows-nat.c (thread_rec)
	(windows_nat_target::fetch_registers): Update.
	* nat/windows-nat.h (struct windows_thread_info) <suspended>:
	Update comment.
	<debug_registers_changed, reload_context>: Now bool.

gdb/gdbserver/ChangeLog
2019-11-26  Tom Tromey  <tromey@adacore.com>

	* win32-i386-low.c (update_debug_registers)
	(i386_prepare_to_resume, i386_thread_added): Update.

Change-Id: I76e2a3a11543fde4fbd24491d96c2087de1b98b7
---
M gdb/ChangeLog
M gdb/gdbserver/ChangeLog
M gdb/gdbserver/win32-i386-low.c
M gdb/nat/windows-nat.h
M gdb/windows-nat.c
5 files changed, 24 insertions(+), 8 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 07b5cee..e48f650 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2019-11-26  Tom Tromey  <tromey@adacore.com>
 
+	* windows-nat.c (thread_rec)
+	(windows_nat_target::fetch_registers): Update.
+	* nat/windows-nat.h (struct windows_thread_info) <suspended>:
+	Update comment.
+	<debug_registers_changed, reload_context>: Now bool.
+
+2019-11-26  Tom Tromey  <tromey@adacore.com>
+
 	* windows-nat.c (windows_add_thread): Use new.
 	(windows_init_thread_list, windows_delete_thread): Use delete.
 	(get_windows_debug_event): Update.
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index e376af3..2ebf068 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,10 @@
 2019-11-26  Tom Tromey  <tromey@adacore.com>
 
+	* win32-i386-low.c (update_debug_registers)
+	(i386_prepare_to_resume, i386_thread_added): Update.
+
+2019-11-26  Tom Tromey  <tromey@adacore.com>
+
 	* win32-low.c (child_add_thread): Use new.
 	(delete_thread_info): Use delete.
 
diff --git a/gdb/gdbserver/win32-i386-low.c b/gdb/gdbserver/win32-i386-low.c
index 3fc0cf1..b834b16 100644
--- a/gdb/gdbserver/win32-i386-low.c
+++ b/gdb/gdbserver/win32-i386-low.c
@@ -44,7 +44,7 @@
 
   /* The actual update is done later just before resuming the lwp,
      we just mark that the registers need updating.  */
-  th->debug_registers_changed = 1;
+  th->debug_registers_changed = true;
 }
 
 /* Update the inferior's debug register REGNUM from STATE.  */
@@ -253,14 +253,14 @@
 	 FIXME: should we set dr6 also ?? */
       th->context.Dr7 = dr->dr_control_mirror;
 
-      th->debug_registers_changed = 0;
+      th->debug_registers_changed = false;
     }
 }
 
 static void
 i386_thread_added (windows_thread_info *th)
 {
-  th->debug_registers_changed = 1;
+  th->debug_registers_changed = true;
 }
 
 static void
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 2fe2a2f..e5d7e63 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -46,7 +46,10 @@
   /* Thread Information Block address.  */
   CORE_ADDR thread_local_base;
 
-  /* Non zero if SuspendThread was called on this thread.  */
+  /* This keeps track of whether SuspendThread was called on this
+     thread.  -1 means there was a failure or that the thread was
+     explicitly not suspended, 1 means it was called, and 0 means it
+     was not.  */
   int suspended = 0;
 
 #ifdef _WIN32_WCE
@@ -59,11 +62,11 @@
 
   /* Whether debug registers changed since we last set CONTEXT back to
      the thread.  */
-  int debug_registers_changed = 0;
+  bool debug_registers_changed = false;
 
   /* Nonzero if CONTEXT is invalidated and must be re-read from the
      inferior thread.  */
-  int reload_context = 0;
+  bool reload_context = false;
 
   /* The name of the thread, allocated by xmalloc.  */
   char *name = nullptr;
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 31039d5..4898fba 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -400,7 +400,7 @@
 	      }
 	    else if (get_context < 0)
 	      th->suspended = -1;
-	    th->reload_context = 1;
+	    th->reload_context = true;
 	  }
 	return th;
       }
@@ -604,7 +604,7 @@
 	      dr[7] = th->context.Dr7;
 	    }
 	}
-      th->reload_context = 0;
+      th->reload_context = false;
     }
 
   if (r < 0)

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I76e2a3a11543fde4fbd24491d96c2087de1b98b7
Gerrit-Change-Number: 409
Gerrit-PatchSet: 2
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: newpatchset

[review v2] Change two windows_thread_info members to "bool"

[Pedro Alves (Code Review)]

Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/409
......................................................................


Patch Set 2: Code-Review+2


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I76e2a3a11543fde4fbd24491d96c2087de1b98b7
Gerrit-Change-Number: 409
Gerrit-PatchSet: 2
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Fri, 29 Nov 2019 18:00:51 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment