[3/3] gdbserver: Remove thread_to_gdb_id
Commit Message
As explained in the previous patch, the gdb_id concept is no longer
relevant. The function thread_to_gdb_id is trivial, it returns the
thread's ptid. Remove it and replace its usage with ptid_of.
The changes in nto-low.c and lynx-low.c are fairly straightforward, but
I was not able to build test them.
gdb/gdbserver/ChangeLog:
* inferiors.h (thread_to_gdb_id): Remove.
* inferiors.c (thread_to_gdb_id): Remove.
* server.c (handle_qxfer_threads_worker, handle_query): Adjust.
* lynx-low.c (lynx_resume, lynx_wait_1, lynx_fetch_registers,
lynx_store_registers, lynx_read_memory, lynx_write_memory):
Likewise.
* nto-low.c (nto_fetch_registers, nto_store_registers,
nto_stopped_by_watchpoint, nto_stopped_data_address): Likewise.
---
gdb/gdbserver/inferiors.c | 6 ------
gdb/gdbserver/inferiors.h | 2 --
gdb/gdbserver/lynx-low.c | 14 +++++++-------
gdb/gdbserver/nto-low.c | 8 ++++----
gdb/gdbserver/server.c | 22 +++++++++-------------
5 files changed, 20 insertions(+), 32 deletions(-)
Comments
On 09/10/2017 09:11 PM, Simon Marchi wrote:
> As explained in the previous patch, the gdb_id concept is no longer
> relevant. The function thread_to_gdb_id is trivial, it returns the
> thread's ptid. Remove it and replace its usage with ptid_of.
>
> The changes in nto-low.c and lynx-low.c are fairly straightforward, but
> I was not able to build test them.
Seems fine to me. Nits below.
> @@ -869,7 +869,7 @@ nto_stopped_by_watchpoint (void)
> {
> ptid_t ptid;
>
> - ptid = thread_to_gdb_id (current_thread);
> + ptid = ptid_of (current_thread);
> if (nto_set_thread (ptid))
> {
> const int watchmask = _DEBUG_FLAG_TRACE_RD | _DEBUG_FLAG_TRACE_WR
> @@ -901,7 +901,7 @@ nto_stopped_data_address (void)
> {
> ptid_t ptid;
>
> - ptid = thread_to_gdb_id (current_thread);
> + ptid = ptid_of (current_thread);
>
Above two hunks could merge decl + init.
> @@ -2070,21 +2070,21 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
> /* Reply the current thread id. */
> if (strcmp ("qC", own_buf) == 0 && !disable_packet_qC)
> {
> - ptid_t gdb_id;
> + ptid_t ptid;
> require_running (own_buf);
>
> if (!ptid_equal (general_thread, null_ptid)
> && !ptid_equal (general_thread, minus_one_ptid))
In the other patch you converted equivalent checks to use op!= ,
I don't mind doing it here as well while at it:
if (general_thread != null_ptid
&& general_thread != minus_one_ptid)
> - gdb_id = general_thread;
> + ptid = general_thread;
> else
> {
> thread_ptr = get_first_inferior (&all_threads);
> - gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
> + ptid = thread_ptr->id;
> }
>
> sprintf (own_buf, "QC");
> own_buf += 2;
> - write_ptid (own_buf, gdb_id);
> + write_ptid (own_buf, ptid);
> return;
> }
>
> @@ -2140,28 +2140,24 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
> {
> if (strcmp ("qfThreadInfo", own_buf) == 0)
> {
> - ptid_t gdb_id;
> -
> require_running (own_buf);
> thread_ptr = get_first_inferior (&all_threads);
>
> *own_buf++ = 'm';
> - gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
> - write_ptid (own_buf, gdb_id);
> + ptid_t ptid = thread_ptr->id;
> + write_ptid (own_buf, ptid);
Could even get rid of the ptid_t variable?
write_ptid (own_buf, thread_ptr->id);
> thread_ptr = thread_ptr->next;
> return;
> }
>
> if (strcmp ("qsThreadInfo", own_buf) == 0)
> {
> - ptid_t gdb_id;
> -
> require_running (own_buf);
> if (thread_ptr != NULL)
> {
> *own_buf++ = 'm';
> - gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
> - write_ptid (own_buf, gdb_id);
> + ptid_t ptid = thread_ptr->id;
> + write_ptid (own_buf, ptid);
Ditto.
> thread_ptr = thread_ptr->next;
> return;
> }
>
Thanks,
Pedro Alves
On 2017-09-15 13:02, Pedro Alves wrote:
> On 09/10/2017 09:11 PM, Simon Marchi wrote:
>> As explained in the previous patch, the gdb_id concept is no longer
>> relevant. The function thread_to_gdb_id is trivial, it returns the
>> thread's ptid. Remove it and replace its usage with ptid_of.
>>
>> The changes in nto-low.c and lynx-low.c are fairly straightforward,
>> but
>> I was not able to build test them.
>
> Seems fine to me. Nits below.
>
>> @@ -869,7 +869,7 @@ nto_stopped_by_watchpoint (void)
>> {
>> ptid_t ptid;
>>
>> - ptid = thread_to_gdb_id (current_thread);
>> + ptid = ptid_of (current_thread);
>> if (nto_set_thread (ptid))
>> {
>> const int watchmask = _DEBUG_FLAG_TRACE_RD | _DEBUG_FLAG_TRACE_WR
>> @@ -901,7 +901,7 @@ nto_stopped_data_address (void)
>> {
>> ptid_t ptid;
>>
>> - ptid = thread_to_gdb_id (current_thread);
>> + ptid = ptid_of (current_thread);
>>
>
> Above two hunks could merge decl + init.
In the files I can't build, I usually try to be as unintrusive as
possible, but I'm fine with doing this, it's obvious (tm).
>> @@ -2070,21 +2070,21 @@ handle_query (char *own_buf, int packet_len,
>> int *new_packet_len_p)
>> /* Reply the current thread id. */
>> if (strcmp ("qC", own_buf) == 0 && !disable_packet_qC)
>> {
>> - ptid_t gdb_id;
>> + ptid_t ptid;
>> require_running (own_buf);
>>
>> if (!ptid_equal (general_thread, null_ptid)
>> && !ptid_equal (general_thread, minus_one_ptid))
>
> In the other patch you converted equivalent checks to use op!= ,
> I don't mind doing it here as well while at it:
>
> if (general_thread != null_ptid
> && general_thread != minus_one_ptid)
>
>
>> - gdb_id = general_thread;
>> + ptid = general_thread;
>> else
>> {
>> thread_ptr = get_first_inferior (&all_threads);
>> - gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
>> + ptid = thread_ptr->id;
>> }
>>
>> sprintf (own_buf, "QC");
>> own_buf += 2;
>> - write_ptid (own_buf, gdb_id);
>> + write_ptid (own_buf, ptid);
>> return;
>> }
>>
>> @@ -2140,28 +2140,24 @@ handle_query (char *own_buf, int packet_len,
>> int *new_packet_len_p)
>> {
>> if (strcmp ("qfThreadInfo", own_buf) == 0)
>> {
>> - ptid_t gdb_id;
>> -
>> require_running (own_buf);
>> thread_ptr = get_first_inferior (&all_threads);
>>
>> *own_buf++ = 'm';
>> - gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
>> - write_ptid (own_buf, gdb_id);
>> + ptid_t ptid = thread_ptr->id;
>> + write_ptid (own_buf, ptid);
>
> Could even get rid of the ptid_t variable?
>
> write_ptid (own_buf, thread_ptr->id);
>
>> thread_ptr = thread_ptr->next;
>> return;
>> }
>>
>> if (strcmp ("qsThreadInfo", own_buf) == 0)
>> {
>> - ptid_t gdb_id;
>> -
>> require_running (own_buf);
>> if (thread_ptr != NULL)
>> {
>> *own_buf++ = 'm';
>> - gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
>> - write_ptid (own_buf, gdb_id);
>> + ptid_t ptid = thread_ptr->id;
>> + write_ptid (own_buf, ptid);
>
> Ditto.
>
>> thread_ptr = thread_ptr->next;
>> return;
>> }
>>
Ack everything. The changes are small enough that I don't think it
warrants a v2. I'll push directly once the handle_detach refactor is in
(and post the final versions here).
Thanks!
Simon
On 09/15/2017 02:54 PM, Simon Marchi wrote:
>
> Ack everything. The changes are small enough that I don't think it
> warrants a v2. I'll push directly once the handle_detach refactor is in
> (and post the final versions here).
>
> Thanks!
Agreed, that's fine with me.
Thanks,
Pedro Alves
@@ -121,12 +121,6 @@ add_thread (ptid_t thread_id, void *target_data)
return new_thread;
}
-ptid_t
-thread_to_gdb_id (struct thread_info *thread)
-{
- return thread->entry.id;
-}
-
/* Wrapper around get_first_inferior to return a struct thread_info *. */
struct thread_info *
@@ -143,8 +143,6 @@ struct process_info *find_process_pid (int pid);
int have_started_inferiors_p (void);
int have_attached_inferiors_p (void);
-ptid_t thread_to_gdb_id (struct thread_info *);
-
void clear_inferiors (void);
struct inferior_list_entry *find_inferior
(struct inferior_list *,
@@ -350,7 +350,7 @@ lynx_resume (struct thread_resume *resume_info, size_t n)
the moment we resume its execution for the first time. It is
fine to use the current_thread's ptid in those cases. */
if (ptid_equal (ptid, minus_one_ptid))
- ptid = thread_to_gdb_id (current_thread);
+ ptid = ptid_of (current_thread);
regcache_invalidate_pid (ptid_get_pid (ptid));
@@ -423,7 +423,7 @@ lynx_wait_1 (ptid_t ptid, struct target_waitstatus *status, int options)
ptid_t new_ptid;
if (ptid_equal (ptid, minus_one_ptid))
- pid = lynx_ptid_get_pid (thread_to_gdb_id (current_thread));
+ pid = lynx_ptid_get_pid (ptid_of (current_thread));
else
pid = BUILDPID (lynx_ptid_get_pid (ptid), lynx_ptid_get_tid (ptid));
@@ -612,7 +612,7 @@ static void
lynx_fetch_registers (struct regcache *regcache, int regno)
{
struct lynx_regset_info *regset = lynx_target_regsets;
- ptid_t inferior_ptid = thread_to_gdb_id (current_thread);
+ ptid_t inferior_ptid = ptid_of (current_thread);
lynx_debug ("lynx_fetch_registers (regno = %d)", regno);
@@ -637,7 +637,7 @@ static void
lynx_store_registers (struct regcache *regcache, int regno)
{
struct lynx_regset_info *regset = lynx_target_regsets;
- ptid_t inferior_ptid = thread_to_gdb_id (current_thread);
+ ptid_t inferior_ptid = ptid_of (current_thread);
lynx_debug ("lynx_store_registers (regno = %d)", regno);
@@ -673,7 +673,7 @@ lynx_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
int buf;
const int xfer_size = sizeof (buf);
CORE_ADDR addr = memaddr & -(CORE_ADDR) xfer_size;
- ptid_t inferior_ptid = thread_to_gdb_id (current_thread);
+ ptid_t inferior_ptid = ptid_of (current_thread);
while (addr < memaddr + len)
{
@@ -706,7 +706,7 @@ lynx_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
int buf;
const int xfer_size = sizeof (buf);
CORE_ADDR addr = memaddr & -(CORE_ADDR) xfer_size;
- ptid_t inferior_ptid = thread_to_gdb_id (current_thread);
+ ptid_t inferior_ptid = ptid_of (current_thread);
while (addr < memaddr + len)
{
@@ -742,7 +742,7 @@ lynx_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
static void
lynx_request_interrupt (void)
{
- ptid_t inferior_ptid = thread_to_gdb_id (get_first_thread ());
+ ptid_t inferior_ptid = ptid_of (get_first_thread ());
kill (lynx_ptid_get_pid (inferior_ptid), SIGINT);
}
@@ -631,7 +631,7 @@ nto_fetch_registers (struct regcache *regcache, int regno)
TRACE ("current_thread is NULL\n");
return;
}
- ptid = thread_to_gdb_id (current_thread);
+ ptid = ptid_of (current_thread);
if (!nto_set_thread (ptid))
return;
@@ -678,7 +678,7 @@ nto_store_registers (struct regcache *regcache, int regno)
TRACE ("current_thread is NULL\n");
return;
}
- ptid = thread_to_gdb_id (current_thread);
+ ptid = ptid_of (current_thread);
if (!nto_set_thread (ptid))
return;
@@ -869,7 +869,7 @@ nto_stopped_by_watchpoint (void)
{
ptid_t ptid;
- ptid = thread_to_gdb_id (current_thread);
+ ptid = ptid_of (current_thread);
if (nto_set_thread (ptid))
{
const int watchmask = _DEBUG_FLAG_TRACE_RD | _DEBUG_FLAG_TRACE_WR
@@ -901,7 +901,7 @@ nto_stopped_data_address (void)
{
ptid_t ptid;
- ptid = thread_to_gdb_id (current_thread);
+ ptid = ptid_of (current_thread);
if (nto_set_thread (ptid))
{
@@ -1586,7 +1586,7 @@ handle_qxfer_threads_worker (struct inferior_list_entry *inf, void *arg)
{
struct thread_info *thread = (struct thread_info *) inf;
struct buffer *buffer = (struct buffer *) arg;
- ptid_t ptid = thread_to_gdb_id (thread);
+ ptid_t ptid = ptid_of (thread);
char ptid_s[100];
int core = target_core_of_thread (ptid);
char core_s[21];
@@ -2070,21 +2070,21 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
/* Reply the current thread id. */
if (strcmp ("qC", own_buf) == 0 && !disable_packet_qC)
{
- ptid_t gdb_id;
+ ptid_t ptid;
require_running (own_buf);
if (!ptid_equal (general_thread, null_ptid)
&& !ptid_equal (general_thread, minus_one_ptid))
- gdb_id = general_thread;
+ ptid = general_thread;
else
{
thread_ptr = get_first_inferior (&all_threads);
- gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
+ ptid = thread_ptr->id;
}
sprintf (own_buf, "QC");
own_buf += 2;
- write_ptid (own_buf, gdb_id);
+ write_ptid (own_buf, ptid);
return;
}
@@ -2140,28 +2140,24 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
{
if (strcmp ("qfThreadInfo", own_buf) == 0)
{
- ptid_t gdb_id;
-
require_running (own_buf);
thread_ptr = get_first_inferior (&all_threads);
*own_buf++ = 'm';
- gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
- write_ptid (own_buf, gdb_id);
+ ptid_t ptid = thread_ptr->id;
+ write_ptid (own_buf, ptid);
thread_ptr = thread_ptr->next;
return;
}
if (strcmp ("qsThreadInfo", own_buf) == 0)
{
- ptid_t gdb_id;
-
require_running (own_buf);
if (thread_ptr != NULL)
{
*own_buf++ = 'm';
- gdb_id = thread_to_gdb_id ((struct thread_info *)thread_ptr);
- write_ptid (own_buf, gdb_id);
+ ptid_t ptid = thread_ptr->id;
+ write_ptid (own_buf, ptid);
thread_ptr = thread_ptr->next;
return;
}