public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 2/2] gdb: use -1 for breakpoint::task default value
Date: Wed,  8 Feb 2023 15:16:17 +0000	[thread overview]
Message-ID: <6ab8502906e90c53dbb2a758f2e02acab05da4ee.1675869267.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1675869267.git.aburgess@redhat.com>

Within the breakpoint struct we have two fields ::thread and ::task
which are used for thread or task specific breakpoints.  When a
breakpoint doesn't have a specific thread or task then these fields
have the values -1 and 0 respectively.

There's no particular reason (as far as I can tell) why these two
"default" values are different, and I find the difference a little
confusing.  Long term I'd like to potentially fold these two fields
into a single field, but that isn't what this commit does.

What this commit does is switch to using -1 as the "default" value for
both fields, this means that the default for breakpoint::task has
changed from 0 to -1.   I've updated all the code I can find that
relied on the value of 0, and I see no test regressions, especially in
gdb.ada/tasks.exp, which still fully passes.

There should be no user visible changes after this commit.
---
 gdb/breakpoint.c           | 36 ++++++++++++++++++------------------
 gdb/breakpoint.h           | 10 +++++-----
 gdb/guile/scm-breakpoint.c |  6 +++---
 gdb/python/py-breakpoint.c |  6 +++---
 4 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8963b10d516..4792bc8263d 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1462,7 +1462,7 @@ breakpoint_set_thread (struct breakpoint *b, int thread)
   /* It is invalid to set the thread field to anything other than -1 (which
      means no thread restriction) if a task restriction is already in
      place.  */
-  gdb_assert (thread == -1 || b->task == 0);
+  gdb_assert (thread == -1 || b->task == -1);
 
   int old_thread = b->thread;
 
@@ -1476,10 +1476,10 @@ breakpoint_set_thread (struct breakpoint *b, int thread)
 void
 breakpoint_set_task (struct breakpoint *b, int task)
 {
-  /* It is invalid to set the task field to anything other than 0 (which
+  /* It is invalid to set the task field to anything other than -1 (which
      means no task restriction) if a thread restriction is already in
      place.  */
-  gdb_assert (task == 0 || b->thread == -1);
+  gdb_assert (task == -1 || b->thread == -1);
 
   int old_task = b->task;
 
@@ -5473,7 +5473,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
      evaluating the condition if this isn't the specified
      thread/task.  */
   if ((b->thread != -1 && b->thread != thread->global_num)
-      || (b->task != 0 && b->task != ada_get_task_number (thread)))
+      || (b->task != -1 && b->task != ada_get_task_number (thread)))
     {
       infrun_debug_printf ("incorrect thread or task, not stopping");
       bs->stop = false;
@@ -6487,7 +6487,7 @@ print_one_breakpoint_location (struct breakpoint *b,
     {
       if (b->thread != -1)
 	uiout->field_signed ("thread", b->thread);
-      else if (b->task != 0)
+      else if (b->task != -1)
 	uiout->field_signed ("task", b->task);
     }
 
@@ -6544,7 +6544,7 @@ print_one_breakpoint_location (struct breakpoint *b,
       uiout->text ("\n");
     }
 
-  if (!part_of_multiple && b->task != 0)
+  if (!part_of_multiple && b->task != -1)
     {
       uiout->text ("\tstop only in task ");
       uiout->field_signed ("task", b->task);
@@ -8447,7 +8447,7 @@ code_breakpoint::code_breakpoint (struct gdbarch *gdbarch_,
   gdb_assert (!sals.empty ());
 
   /* At most one of thread or task can be set on any breakpoint.  */
-  gdb_assert (thread == -1 || task == 0);
+  gdb_assert (thread == -1 || task == -1);
   thread = thread_;
   task = task_;
 
@@ -8763,7 +8763,7 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc,
 {
   cond_string->reset ();
   *thread = -1;
-  *task = 0;
+  *task = -1;
   rest->reset ();
   bool force = false;
 
@@ -8816,7 +8816,7 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc,
 	  if (*thread != -1)
 	    error(_("You can specify only one thread."));
 
-	  if (*task != 0)
+	  if (*task != -1)
 	    error (_("You can specify only one of thread or task."));
 
 	  tok = end_tok + 1;
@@ -8830,7 +8830,7 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc,
 	{
 	  char *tmptok;
 
-	  if (*task != 0)
+	  if (*task != -1)
 	    error(_("You can specify only one task."));
 
 	  if (*thread != -1)
@@ -8871,7 +8871,7 @@ find_condition_and_thread_for_sals (const std::vector<symtab_and_line> &sals,
     {
       gdb::unique_xmalloc_ptr<char> cond;
       int thread_id = -1;
-      int task_id = 0;
+      int task_id = -1;
       gdb::unique_xmalloc_ptr<char> remaining;
 
       /* Here we want to parse 'arg' to separate condition from thread
@@ -8886,7 +8886,7 @@ find_condition_and_thread_for_sals (const std::vector<symtab_and_line> &sals,
 				     &task_id, &remaining);
 	  *cond_string = std::move (cond);
 	  /* At most one of thread or task can be set.  */
-	  gdb_assert (thread_id == -1 || task_id == 0);
+	  gdb_assert (thread_id == -1 || task_id == -1);
 	  *thread = thread_id;
 	  *task = task_id;
 	  *rest = std::move (remaining);
@@ -8988,7 +8988,7 @@ create_breakpoint (struct gdbarch *gdbarch,
 {
   struct linespec_result canonical;
   bool pending = false;
-  int task = 0;
+  int task = -1;
   int prev_bkpt_count = breakpoint_count;
 
   gdb_assert (ops != NULL);
@@ -10060,7 +10060,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
      the hardware watchpoint.  */
   bool use_mask = false;
   CORE_ADDR mask = 0;
-  int task = 0;
+  int task = -1;
 
   /* Make sure that we actually have parameters to parse.  */
   if (arg != NULL && arg[0] != '\0')
@@ -10107,7 +10107,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
 	      if (thread != -1)
 		error(_("You can specify only one thread."));
 
-	      if (task != 0)
+	      if (task != -1)
 		error (_("You can specify only one of thread or task."));
 
 	      /* Extract the thread ID from the next token.  */
@@ -10123,7 +10123,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
 	    {
 	      char *tmp;
 
-	      if (task != 0)
+	      if (task != -1)
 		error(_("You can specify only one task."));
 
 	      if (thread != -1)
@@ -10305,7 +10305,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
     w.reset (new watchpoint (nullptr, bp_type));
 
   /* At most one of thread or task can be set on a watchpoint.  */
-  gdb_assert (thread == -1 || task == 0);
+  gdb_assert (thread == -1 || task == -1);
   w->thread = thread;
   w->task = task;
   w->disposition = disp_donttouch;
@@ -14143,7 +14143,7 @@ breakpoint::print_recreate_thread (struct ui_file *fp) const
   if (thread != -1)
     gdb_printf (fp, " thread %d", thread);
 
-  if (task != 0)
+  if (task != -1)
     gdb_printf (fp, " task %d", task);
 
   gdb_printf (fp, "\n");
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 5228c38fe02..03aecd15eff 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -802,9 +802,9 @@ struct breakpoint
      care.  */
   int thread = -1;
 
-  /* Ada task number for task-specific breakpoint, or 0 if don't
+  /* Ada task number for task-specific breakpoint, or -1 if don't
      care.  */
-  int task = 0;
+  int task = -1;
 
   /* Count of the number of times this breakpoint was taken, dumped
      with the info, but not used for anything else.  Useful for seeing
@@ -1680,9 +1680,9 @@ extern void breakpoint_set_silent (struct breakpoint *b, int silent);
 
 extern void breakpoint_set_thread (struct breakpoint *b, int thread);
 
-/* Set the task for this breakpoint.  If TASK is 0, make the breakpoint
-   work for any task.  Passing a value other than 0 for TASK should only be
-   done if b->thread is -1; it is not valid to try and set both a thread
+/* Set the task for this breakpoint.  If TASK is -1, make the breakpoint
+   work for any task.  Passing a value other than -1 for TASK should only
+   be done if b->thread is -1; it is not valid to try and set both a thread
    and task restriction on a breakpoint.  */
 
 extern void breakpoint_set_task (struct breakpoint *b, int task);
diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index d4f2b7310bd..2931df265d7 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -774,7 +774,7 @@ gdbscm_set_breakpoint_thread_x (SCM self, SCM newvalue)
 				     _("invalid thread id"));
 	}
 
-      if (bp_smob->bp->task != 0)
+      if (bp_smob->bp->task != -1)
 	scm_misc_error (FUNC_NAME,
 			_("cannot set both task and thread attributes"),
 			SCM_EOL);
@@ -797,7 +797,7 @@ gdbscm_breakpoint_task (SCM self)
   breakpoint_smob *bp_smob
     = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
-  if (bp_smob->bp->task == 0)
+  if (bp_smob->bp->task == -1)
     return SCM_BOOL_F;
 
   return scm_from_long (bp_smob->bp->task);
@@ -840,7 +840,7 @@ gdbscm_set_breakpoint_task_x (SCM self, SCM newvalue)
 			SCM_EOL);
     }
   else if (gdbscm_is_false (newvalue))
-    id = 0;
+    id = -1;
   else
     SCM_ASSERT_TYPE (0, newvalue, SCM_ARG2, FUNC_NAME, _("integer or #f"));
 
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 52298935242..ecf52a4637c 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -271,7 +271,7 @@ bppy_set_thread (PyObject *self, PyObject *newvalue, void *closure)
 	  return -1;
 	}
 
-      if (self_bp->bp->task != 0)
+      if (self_bp->bp->task != -1)
 	{
 	  PyErr_SetString (PyExc_RuntimeError,
 			   _("Cannot set both task and thread attributes."));
@@ -337,7 +337,7 @@ bppy_set_task (PyObject *self, PyObject *newvalue, void *closure)
 	}
     }
   else if (newvalue == Py_None)
-    id = 0;
+    id = -1;
   else
     {
       PyErr_SetString (PyExc_TypeError,
@@ -711,7 +711,7 @@ bppy_get_task (PyObject *self, void *closure)
 
   BPPY_REQUIRE_VALID (self_bp);
 
-  if (self_bp->bp->task == 0)
+  if (self_bp->bp->task == -1)
     Py_RETURN_NONE;
 
   return gdb_py_object_from_longest (self_bp->bp->task).release ();
-- 
2.25.4


  parent reply	other threads:[~2023-02-08 15:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-08 15:16 [PATCH 0/2] Prevent combining 'task' and 'thread' keywords Andrew Burgess
2023-02-08 15:16 ` [PATCH 1/2] gdb: only allow one of thread or task on breakpoints or watchpoints Andrew Burgess
2023-02-08 15:52   ` Eli Zaretskii
2023-02-08 18:10   ` Pedro Alves
2023-02-12  5:50     ` Andrew Burgess
2023-02-12 22:58       ` Tom Tromey
2023-02-13 11:26         ` Andrew Burgess
2023-02-08 15:16 ` Andrew Burgess [this message]
2023-02-08 18:10   ` [PATCH 2/2] gdb: use -1 for breakpoint::task default value Pedro Alves
2023-02-12  7:23     ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6ab8502906e90c53dbb2a758f2e02acab05da4ee.1675869267.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).