This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/3] gdbserver: Remove gdb_id_to_thread_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 11:55:10 +0100
- Subject: Re: [PATCH 2/3] gdbserver: Remove gdb_id_to_thread_id
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx05.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 D3CF0552F1
- References: <1505074275-1662-1-git-send-email-simon.marchi@ericsson.com> <1505074275-1662-3-git-send-email-simon.marchi@ericsson.com>
On 09/10/2017 09:11 PM, Simon Marchi wrote:
> From what I understand, this function is not doing anything useful as of
> today.
>
> Here's the result of my archeological research:
>
*nod*
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -4011,7 +4011,6 @@ process_serial_event (void)
> unsigned int len;
> int res;
> CORE_ADDR mem_addr;
> - int pid;
> unsigned char sig;
> int packet_len;
> int new_packet_len = -1;
> @@ -4039,92 +4038,96 @@ process_serial_event (void)
> handle_general_set (own_buf);
> break;
> case 'D':
> - require_running (own_buf);
> + {
> + require_running (own_buf);
>
The reindentation makes it hard to see the actual
change. Is it just moving the int pid variable, or something else?
IMO, it'd be nicer to move the whole case 'D' body to a
handle_detach function.
> case '!':
> extended_protocol = 1;
> write_ok (own_buf);
> @@ -4135,23 +4138,18 @@ process_serial_event (void)
> case 'H':
> if (own_buf[1] == 'c' || own_buf[1] == 'g' || own_buf[1] == 's')
> {
> - ptid_t gdb_id, thread_id;
> - int pid;
> + ptid_t thread_id;
>
> require_running (own_buf);
>
> - gdb_id = read_ptid (&own_buf[2], NULL);
> -
> - pid = ptid_get_pid (gdb_id);
> + thread_id = read_ptid (&own_buf[2], NULL);
Move ptid_t declaration here? :
ptid_t thread_id = read_ptid (&own_buf[2], NULL);
>
> - if (ptid_equal (gdb_id, null_ptid)
> - || ptid_equal (gdb_id, minus_one_ptid))
> + if (thread_id == null_ptid || thread_id == minus_one_ptid)
> thread_id = null_ptid;
This can be just:
if (thread_id == minus_one_ptid)
thread_id = null_ptid;
> - else if (pid != 0
> - && ptid_equal (pid_to_ptid (pid),
> - gdb_id))
> + else if (thread_id.is_pid ())
> {
> - thread_info *thread = find_any_thread_of_pid (pid);
> + /* The ptid represents a pid. */
> + thread_info *thread = find_any_thread_of_pid (thread_id.pid ());
>
> if (thread == NULL)
> {
> @@ -4163,8 +4161,8 @@ process_serial_event (void)
> }
> else
> {
> - thread_id = gdb_id_to_thread_id (gdb_id);
> - if (ptid_equal (thread_id, null_ptid))
> + /* The ptid represents a lwp/tid. */
> + if (find_thread_ptid (thread_id) == NULL)
> {
> write_enn (own_buf);
> break;
> @@ -4369,13 +4367,12 @@ process_serial_event (void)
>
> case 'T':
> {
> - ptid_t gdb_id, thread_id;
> + ptid_t thread_id;
>
> require_running (own_buf);
>
> - gdb_id = read_ptid (&own_buf[1], NULL);
> - thread_id = gdb_id_to_thread_id (gdb_id);
> - if (ptid_equal (thread_id, null_ptid))
> + thread_id = read_ptid (&own_buf[1], NULL);
ptid_t thread_id = ... ?
> + if (find_thread_ptid (thread_id) == NULL)
> {
> write_enn (own_buf);
> break;
>
Thanks,
Pedro Alves