diff mbox

[2/8] Check LWP_SIGNAL_CAN_BE_DELIVERED for enqueue/dequeue pending signals

Message ID 1457088276-1170-3-git-send-email-yao.qi@linaro.org
State New, archived
Headers

Commit Message

Yao Qi March 4, 2016, 10:44 a.m. UTC 
  The enqueue and dequeue signals in linux_resume_one_lwp_throw use one
condition and its inverted one.  This patch is to move the condition
into a macro LWP_SIGNAL_CAN_BE_DELIVERED, so that the next patch can
change the condition in one place.

gdb/gdbserver:

2016-03-04  Yao Qi  <yao.qi@linaro.org>

	* linux-low.c (LWP_SIGNAL_CAN_BE_DELIVERED): New macro.
	(linux_resume_one_lwp_throw): Use LWP_SIGNAL_CAN_BE_DELIVERED.
---
 gdb/gdbserver/linux-low.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

Comments

Pedro Alves March 11, 2016, 10:55 a.m. UTC | #1 
On 03/04/2016 10:44 AM, Yao Qi wrote:
> The enqueue and dequeue signals in linux_resume_one_lwp_throw use one
> condition and its inverted one.  This patch is to move the condition
> into a macro LWP_SIGNAL_CAN_BE_DELIVERED, so that the next patch can
> change the condition in one place.

I like the idea, but why a macro instead of a function?

Thanks,
Pedro Alves
Luis Machado March 16, 2016, 5:02 p.m. UTC | #2 
On 03/04/2016 07:44 AM, Yao Qi wrote:
> The enqueue and dequeue signals in linux_resume_one_lwp_throw use one
> condition and its inverted one.  This patch is to move the condition
> into a macro LWP_SIGNAL_CAN_BE_DELIVERED, so that the next patch can
> change the condition in one place.
>
> gdb/gdbserver:
>
> 2016-03-04  Yao Qi  <yao.qi@linaro.org>
>
> 	* linux-low.c (LWP_SIGNAL_CAN_BE_DELIVERED): New macro.
> 	(linux_resume_one_lwp_throw): Use LWP_SIGNAL_CAN_BE_DELIVERED.
> ---
>   gdb/gdbserver/linux-low.c | 23 +++++++++++++----------
>   1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 4520a4a..dcf58db 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -4118,6 +4118,13 @@ single_step (struct lwp_info* lwp)
>     return step;
>   }
>
> +/* The signal can not be delivered to the inferior if we are trying to
> +   reinsert a breakpoint or we're trying to finish a fast tracepoint
> +   collect.  */
> +
> +#define LWP_SIGNAL_CAN_BE_DELIVERED(LWP) \
> +  !((LWP)->bp_reinsert != 0 || (LWP)->collecting_fast_tracepoint)
> +
>   /* Resume execution of LWP.  If STEP is nonzero, single-step it.  If
>      SIGNAL is nonzero, give it that signal.  */
>

Should we state when a signal can be delivered as opposed to describing 
when it cannot be delivered? Or keep the description and change the 
macro/function to LWP_SIGNAL_CANNOT_BE_DELIVERED(LWP)? Otherwise the 
description won't match the macro/function.

Otherwise looks fine to me.
diff mbox

Patch

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 4520a4a..dcf58db 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4118,6 +4118,13 @@  single_step (struct lwp_info* lwp)
   return step;
 }
 
+/* The signal can not be delivered to the inferior if we are trying to
+   reinsert a breakpoint or we're trying to finish a fast tracepoint
+   collect.  */
+
+#define LWP_SIGNAL_CAN_BE_DELIVERED(LWP) \
+  !((LWP)->bp_reinsert != 0 || (LWP)->collecting_fast_tracepoint)
+
 /* Resume execution of LWP.  If STEP is nonzero, single-step it.  If
    SIGNAL is nonzero, give it that signal.  */
 
@@ -4157,13 +4164,12 @@  linux_resume_one_lwp_throw (struct lwp_info *lwp,
     }
 
   /* If we have pending signals or status, and a new signal, enqueue the
-     signal.  Also enqueue the signal if we are waiting to reinsert a
-     breakpoint; it will be picked up again below.  */
+     signal.  Also enqueue the signal if it can't be delivered to the
+     inferior right now.  */
   if (signal != 0
       && (lwp->status_pending_p
 	  || lwp->pending_signals != NULL
-	  || lwp->bp_reinsert != 0
-	  || fast_tp_collecting))
+	  || !LWP_SIGNAL_CAN_BE_DELIVERED (lwp)))
     {
       enqueue_pending_signal (lwp, signal, info);
 
@@ -4269,12 +4275,9 @@  linux_resume_one_lwp_throw (struct lwp_info *lwp,
 	}
     }
 
-  /* If we have pending signals, consume one unless we are trying to
-     reinsert a breakpoint or we're trying to finish a fast tracepoint
-     collect.  */
-  if (lwp->pending_signals != NULL
-      && lwp->bp_reinsert == 0
-      && fast_tp_collecting == 0)
+  /* If we have pending signals, consume one if it can be delivered to
+     the inferior.  */
+  if (lwp->pending_signals != NULL && LWP_SIGNAL_CAN_BE_DELIVERED (lwp))
     {
       struct pending_signals **p_sig;