From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 99446 invoked by alias); 15 Sep 2017 04:09:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 99433 invoked by uid 89); 15 Sep 2017 04:09:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=PROCESS, 8669 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 15 Sep 2017 04:09:23 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 15CBCC047B95; Fri, 15 Sep 2017 04:09:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 15CBCC047B95 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=sergiodj@redhat.com Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D4855600C0; Fri, 15 Sep 2017 04:09:21 +0000 (UTC) From: Sergio Durigan Junior To: Simon Marchi Cc: Subject: Re: [PATCH 1/3] gdbserver: Remove duplicate functions to find any thread of process References: <1505074275-1662-1-git-send-email-simon.marchi@ericsson.com> <1505074275-1662-2-git-send-email-simon.marchi@ericsson.com> Date: Fri, 15 Sep 2017 04:09:00 -0000 In-Reply-To: <1505074275-1662-2-git-send-email-simon.marchi@ericsson.com> (Simon Marchi's message of "Sun, 10 Sep 2017 22:11:13 +0200") Message-ID: <87efr8txfy.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00395.txt.bz2 On Sunday, September 10 2017, Simon Marchi wrote: > We have about 6 functions/callbacks to find_inferior meant to find a > thread that belongs to a given pid. Remove all but > find_any_thread_of_pid and replace their uses with > find_any_thread_of_pid. Thanks for doing. gdbserver is really confusing when dealing with threads and their data structures, so this patch is a nice step towards a better interface. I think this can go in as is, because it is a self contained cleanup, but I would like us to move yet another step ahead: recently I've implemente gdbserver's version of "switch_to_thread", and I couldn't help but think that a "switch_to_any_thread_of_pid" would be good to have, because that's what's happening most of the time in this patch. I know I have to bite the bullet and convert places where "current_thread = find_thread_ptid (ptid)" is still being used so that "switch_to_thread" is used globally. I intend to submit a patch for that. Anyway, this looks good to me. > gdbserver/ChangeLog: > > * server.c (first_thread_of): Remove. > (process_serial_event): Replace usage of first_thread_of with > find_any_thread_of_pid. > * tracepoint.c (same_process_p): Remove. > (gdb_agent_about_to_close): Replace usage of same_process_p with > find_any_thread_of_pid. > * linux-x86-low.c (same_process_callback): Remove. > (x86_arch_setup_process_callback): Replace usage of > same_process_callback with find_any_thread_of_pid. > * thread-db.c (any_thread_of): Remove. > (switch_to_process): Replace usage of any_thread_of with > find_any_thread_of_pid. > * inferiors.c (thread_pid_matches_callback): Remove. > (find_thread_process): Adjust to use find_any_thread_of_pid. > --- > gdb/gdbserver/inferiors.c | 14 +------------- > gdb/gdbserver/linux-x86-low.c | 15 +-------------- > gdb/gdbserver/server.c | 19 +++---------------- > gdb/gdbserver/thread-db.c | 15 +-------------- > gdb/gdbserver/tracepoint.c | 14 +------------- > 5 files changed, 7 insertions(+), 70 deletions(-) > > diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c > index 3c171a1..2212850 100644 > --- a/gdb/gdbserver/inferiors.c > +++ b/gdb/gdbserver/inferiors.c > @@ -141,25 +141,13 @@ find_thread_ptid (ptid_t ptid) > return (struct thread_info *) find_inferior_id (&all_threads, ptid); > } > > -/* Predicate function for matching thread entry's pid to the given > - pid value passed by address in ARGS. */ > - > -static int > -thread_pid_matches_callback (struct inferior_list_entry *entry, void *args) > -{ > - return (ptid_get_pid (entry->id) == *(pid_t *)args); > -} > - > /* Find a thread associated with the given PROCESS, or NULL if no > such thread exists. */ > > static struct thread_info * > find_thread_process (const struct process_info *const process) > { > - pid_t pid = ptid_get_pid (ptid_of (process)); > - > - return (struct thread_info *) > - find_inferior (&all_threads, thread_pid_matches_callback, &pid); > + return find_any_thread_of_pid (process->entry.id.pid ()); > } > > /* Helper for find_any_thread_of_pid. Returns true if a thread > diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c > index 844a165..49f6b2d 100644 > --- a/gdb/gdbserver/linux-x86-low.c > +++ b/gdb/gdbserver/linux-x86-low.c > @@ -846,17 +846,6 @@ x86_linux_read_description (void) > gdb_assert_not_reached ("failed to return tdesc"); > } > > -/* Callback for find_inferior. Stops iteration when a thread with a > - given PID is found. */ > - > -static int > -same_process_callback (struct inferior_list_entry *entry, void *data) > -{ > - int pid = *(int *) data; > - > - return (ptid_get_pid (entry->id) == pid); > -} > - > /* Callback for for_each_inferior. Calls the arch_setup routine for > each process. */ > > @@ -866,9 +855,7 @@ x86_arch_setup_process_callback (struct inferior_list_entry *entry) > int pid = ptid_get_pid (entry->id); > > /* Look up any thread of this processes. */ > - current_thread > - = (struct thread_info *) find_inferior (&all_threads, > - same_process_callback, &pid); > + current_thread = find_any_thread_of_pid (pid); > > the_low_target.arch_setup (); > } > diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c > index 56c6393..bedb87b 100644 > --- a/gdb/gdbserver/server.c > +++ b/gdb/gdbserver/server.c > @@ -3450,17 +3450,6 @@ gdbserver_show_disableable (FILE *stream) > break; \ > } > > -static int > -first_thread_of (struct inferior_list_entry *entry, void *args) > -{ > - int pid = * (int *) args; > - > - if (ptid_get_pid (entry->id) == pid) > - return 1; > - > - return 0; > -} > - > static void > kill_inferior_callback (struct inferior_list_entry *entry) > { > @@ -4162,11 +4151,9 @@ process_serial_event (void) > && ptid_equal (pid_to_ptid (pid), > gdb_id)) > { > - struct thread_info *thread = > - (struct thread_info *) find_inferior (&all_threads, > - first_thread_of, > - &pid); > - if (!thread) > + thread_info *thread = find_any_thread_of_pid (pid); > + > + if (thread == NULL) > { > write_enn (own_buf); > break; > diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c > index 1ffb79d..9b867b8 100644 > --- a/gdb/gdbserver/thread-db.c > +++ b/gdb/gdbserver/thread-db.c > @@ -733,25 +733,12 @@ thread_db_init (void) > return 0; > } > > -static int > -any_thread_of (struct inferior_list_entry *entry, void *args) > -{ > - int *pid_p = (int *) args; > - > - if (ptid_get_pid (entry->id) == *pid_p) > - return 1; > - > - return 0; > -} > - > static void > switch_to_process (struct process_info *proc) > { > int pid = pid_of (proc); > > - current_thread = > - (struct thread_info *) find_inferior (&all_threads, > - any_thread_of, &pid); > + current_thread = find_any_thread_of_pid (pid); > } > > /* Disconnect from libthread_db and free resources. */ > diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c > index 68ce10f..0f41ff4 100644 > --- a/gdb/gdbserver/tracepoint.c > +++ b/gdb/gdbserver/tracepoint.c > @@ -3956,17 +3956,6 @@ cmd_qtstmat (char *packet) > run_inferior_command (packet, strlen (packet) + 1); > } > > -/* Helper for gdb_agent_about_to_close. > - Return non-zero if thread ENTRY is in the same process in DATA. */ > - > -static int > -same_process_p (struct inferior_list_entry *entry, void *data) > -{ > - int *pid = (int *) data; > - > - return ptid_get_pid (entry->id) == *pid; > -} > - > /* Sent the agent a command to close it. */ > > void > @@ -3981,8 +3970,7 @@ gdb_agent_about_to_close (int pid) > saved_thread = current_thread; > > /* Find any thread which belongs to process PID. */ > - current_thread = (struct thread_info *) > - find_inferior (&all_threads, same_process_p, &pid); > + current_thread = find_any_thread_of_pid (pid); > > strcpy (buf, "close"); > > -- > 2.7.4 -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/