Re: [PATCH 2/3] gdbserver lwp_info: Initialize fields, use new/delete

[Simon Marchi <simon.marchi@ericsson.com>]

On 2017-07-25 11:58 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
> 
>> @@ -3480,7 +3476,7 @@ linux_wait_1 (ptid_t ptid,
>>        event_child->collecting_fast_tracepoint
>>  	= linux_fast_tracepoint_collecting (event_child, NULL);
>>  
>> -      if (event_child->collecting_fast_tracepoint != 1)
>> +      if (!event_child->collecting_fast_tracepoint)
>>  	{
>>  	  /* No longer need this breakpoint.  */
>>  	  if (event_child->exit_jump_pad_bkpt != NULL)
> 
> linux_fast_tracepoint_collecting doesn't return boolean, it returns 0, 1
> and 2.   See comments in tracepoint.c:fast_tracepoint_collecting.

Oh, thanks for noticing this!  I think I've been misguided be the comment on
linux_fast_tracepoint_collecting that says "true":

/* Convenience wrapper.  Returns true if LWP is presently collecting a
   fast tracepoint.  */

static int
linux_fast_tracepoint_collecting (struct lwp_info *lwp,
				  struct fast_tpoint_collect_status *status)

Maybe we should make it return an enum so it's clear, because the function name
really sounds like it would return a bool.


Here's the a fixup to this patch that restores the original code:

From 61b9c0b842ef8ecddd0ef8a1be12676d6f9e61ba Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Tue, 25 Jul 2017 12:15:33 +0200
Subject: [PATCH] fixup fast_tracepoint_collecting does not return bool

---
 gdb/gdbserver/linux-low.c | 8 ++++----
 gdb/gdbserver/linux-low.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index e650b0d..ab3e860 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -2177,7 +2177,7 @@ maybe_move_out_of_jump_pad (struct lwp_info *lwp, int *wstat)
 	     reporting to GDB.  Otherwise, it's an IPA lib bug: just
 	     report the signal to GDB, and pray for the best.  */

-	  lwp->collecting_fast_tracepoint = false;
+	  lwp->collecting_fast_tracepoint = 0;

 	  if (r != 0
 	      && (status.adjusted_insn_addr <= lwp->stop_pc
@@ -3476,7 +3476,7 @@ linux_wait_1 (ptid_t ptid,
       event_child->collecting_fast_tracepoint
 	= linux_fast_tracepoint_collecting (event_child, NULL);

-      if (!event_child->collecting_fast_tracepoint)
+      if (event_child->collecting_fast_tracepoint != 1)
 	{
 	  /* No longer need this breakpoint.  */
 	  if (event_child->exit_jump_pad_bkpt != NULL)
@@ -3503,7 +3503,7 @@ linux_wait_1 (ptid_t ptid,
 	    }
 	}

-      if (!event_child->collecting_fast_tracepoint)
+      if (event_child->collecting_fast_tracepoint == 0)
 	{
 	  if (debug_threads)
 	    debug_printf ("fast tracepoint finished "
@@ -5290,7 +5290,7 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except)

   if (thread->last_resume_kind == resume_stop
       && lwp->pending_signals_to_report == NULL
-      && !lwp->collecting_fast_tracepoint)
+      && lwp->collecting_fast_tracepoint == 0)
     {
       /* We haven't reported this LWP as stopped yet (otherwise, the
 	 last_status.kind check above would catch it, and we wouldn't
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index dcc9315..f93aefb 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -358,7 +358,7 @@ struct lwp_info
      return to the jump pad.  Normally, we won't care about this, but
      we will if a signal arrives to this lwp while it is
      collecting.  */
-  bool collecting_fast_tracepoint = false;
+  int collecting_fast_tracepoint = 0;

   /* If this is non-zero, it points to a chain of signals which need
      to be reported to GDB.  These were deferred because the thread
-- 
2.7.4