This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/3] gdbserver: Remove thread_to_gdb_id
- From: Pedro Alves <palves at redhat dot com>
- To: Simon Marchi <simon dot marchi at ericsson dot com>, gdb-patches at sourceware dot org
- Date: Fri, 15 Sep 2017 12:02:39 +0100
- Subject: Re: [PATCH 3/3] gdbserver: Remove thread_to_gdb_id
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 074527E430
- References: <1505074275-1662-1-git-send-email-simon.marchi@ericsson.com> <1505074275-1662-4-git-send-email-simon.marchi@ericsson.com>
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