[PATCH 3/6] gdbserver: Add debugging printf when setting last_resume_kind.

[PATCH 3/6] gdbserver: Add debugging printf when setting last_resume_kind.

[Doug Evans]

Hi.

This patch adds a debugging printf to the setting of last_resume_kind
which I found useful while debugging 16168.

2013-12-17  Doug Evans  <dje@google.com>

	* linux-low.c (resume_kind_to_str): New function.
	(linux_set_resume_request): Add debugging printf when setting
	last_resume_kind.

---
 gdb/gdbserver/linux-low.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 464aaf0..3883e50 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -3301,6 +3301,24 @@ struct thread_resume_array
   size_t n;
 };
 
+/* Return the string name of KIND.  */
+
+static const char *
+resume_kind_to_str (enum resume_kind kind)
+{
+  switch (kind)
+    {
+    case resume_continue:
+      return "resume_continue";
+    case resume_step:
+      return "resume_step";
+    case resume_stop:
+      return "resume_stop";
+    default:
+      gdb_assert_not_reached ("bad resume_kind");
+    }
+}
+
 /* This function is called once per thread via find_inferior.
    ARG is a pointer to a thread_resume_array struct.
    We look up the thread specified by ENTRY in ARG, and mark the thread
@@ -3352,6 +3370,13 @@ linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
 	  lwp->step_range_start = lwp->resume->step_range_start;
 	  lwp->step_range_end = lwp->resume->step_range_end;
 
+	  if (debug_threads)
+	    {
+	      fprintf (stderr, "setting last_resume_kind of LWP %ld to %s\n",
+		       lwpid_of (lwp),
+		       resume_kind_to_str (thread->last_resume_kind));
+	    }
+
 	  /* If we had a deferred signal to report, dequeue one now.
 	     This can happen if LWP gets more than one signal while
 	     trying to get out of a jump pad.  */
-- 
1.8.5.1

Re: [PATCH 3/6] gdbserver: Add debugging printf when setting last_resume_kind.

[Pedro Alves]

On 12/17/2013 09:45 PM, Doug Evans wrote:
> Hi.
> 
> This patch adds a debugging printf to the setting of last_resume_kind
> which I found useful while debugging 16168.
> 
> 2013-12-17  Doug Evans  <dje@google.com>
> 
> 	* linux-low.c (resume_kind_to_str): New function.
> 	(linux_set_resume_request): Add debugging printf when setting
> 	last_resume_kind.
> 
> ---
>  gdb/gdbserver/linux-low.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 464aaf0..3883e50 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -3301,6 +3301,24 @@ struct thread_resume_array
>    size_t n;
>  };
>  
> +/* Return the string name of KIND.  */
> +
> +static const char *
> +resume_kind_to_str (enum resume_kind kind)
> +{
> +  switch (kind)
> +    {
> +    case resume_continue:
> +      return "resume_continue";

Can you do this with a macro please?  See lynx-low.c:ptrace_request_to_str
for example.

OK with that change.

> +    case resume_step:
> +      return "resume_step";
> +    case resume_stop:
> +      return "resume_stop";
> +    default:
> +      gdb_assert_not_reached ("bad resume_kind");
> +    }
> +}
> +


-- 
Pedro Alves

Re: [PATCH 3/6] gdbserver: Add debugging printf when setting last_resume_kind.

[Doug Evans]

On Wed, Dec 18, 2013 at 3:16 AM, Pedro Alves <palves@redhat.com> wrote:
> On 12/17/2013 09:45 PM, Doug Evans wrote:
>> Hi.
>>
>> This patch adds a debugging printf to the setting of last_resume_kind
>> which I found useful while debugging 16168.
>>
>> 2013-12-17  Doug Evans  <dje@google.com>
>>
>>       * linux-low.c (resume_kind_to_str): New function.
>>       (linux_set_resume_request): Add debugging printf when setting
>>       last_resume_kind.
>>
>> ---
>>  gdb/gdbserver/linux-low.c | 25 +++++++++++++++++++++++++
>>  1 file changed, 25 insertions(+)
>>
>> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
>> index 464aaf0..3883e50 100644
>> --- a/gdb/gdbserver/linux-low.c
>> +++ b/gdb/gdbserver/linux-low.c
>> @@ -3301,6 +3301,24 @@ struct thread_resume_array
>>    size_t n;
>>  };
>>
>> +/* Return the string name of KIND.  */
>> +
>> +static const char *
>> +resume_kind_to_str (enum resume_kind kind)
>> +{
>> +  switch (kind)
>> +    {
>> +    case resume_continue:
>> +      return "resume_continue";
>
> Can you do this with a macro please?  See lynx-low.c:ptrace_request_to_str
> for example.
>
> OK with that change.
>
>> +    case resume_step:
>> +      return "resume_step";
>> +    case resume_stop:
>> +      return "resume_stop";
>> +    default:
>> +      gdb_assert_not_reached ("bad resume_kind");
>> +    }
>> +}
>> +

For three values?  Aren't we getting into over-engineering territory?

Re: [PATCH 3/6] gdbserver: Add debugging printf when setting last_resume_kind.

[Pedro Alves]

On 12/18/2013 05:31 PM, Doug Evans wrote:

> For three values?  Aren't we getting into over-engineering territory?

The macro way has the nice property that when reading the code
you never have to wonder "is the string actually matching the
enum name correctly?", so IMO it's worth it even with 3 values.
Plus it's 3 today, but might be more tomorrow.  At which point
will we switch over?  So it might seem that way in isolation,
but I'd rather every place that converts enums to strings
always used the same pattern everywhere.

-- 
Pedro Alves