From patchwork Thu Jun 7 13:54:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 27688 Received: (qmail 68660 invoked by alias); 7 Jun 2018 13:54:34 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 66118 invoked by uid 89); 7 Jun 2018 13:54:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=pulled, owns, Kill X-HELO: gateway23.websitewelcome.com Received: from gateway23.websitewelcome.com (HELO gateway23.websitewelcome.com) (192.185.49.219) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 07 Jun 2018 13:54:28 +0000 Received: from cm12.websitewelcome.com (cm12.websitewelcome.com [100.42.49.8]) by gateway23.websitewelcome.com (Postfix) with ESMTP id A16BEF995 for ; Thu, 7 Jun 2018 08:54:26 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id QvMwfwoomSjJAQvMwfWh48; Thu, 07 Jun 2018 08:54:26 -0500 X-Authority-Reason: nr=8 Received: from 75-166-19-45.hlrn.qwest.net ([75.166.19.45]:33738 helo=bapiya.Home) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1fQvMw-004NwY-8b; Thu, 07 Jun 2018 08:54:26 -0500 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFA] Remove use of queue from remote.c Date: Thu, 7 Jun 2018 07:54:23 -0600 Message-Id: <20180607135423.6763-1-tom@tromey.com> X-BWhitelist: no X-Source-L: No X-Exim-ID: 1fQvMw-004NwY-8b X-Source-Sender: 75-166-19-45.hlrn.qwest.net (bapiya.Home) [75.166.19.45]:33738 X-Source-Auth: tom+tromey.com X-Email-Count: 1 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes This removes a use of the queue data structure (common/queue.h) from remote.c. The queue is replaced with a std::vector. A queue was not needed, as the code never de-queued items. This removes quite a bit of boilerplate code, mostly involved with marshalling arguments to be passed through the queue iterator. Tested by the buildbot. gdb/ChangeLog 2018-06-07 Tom Tromey * remote.c (stop_reply_p): Remove typedef. Don't declare queue. (class remote_state) : Now std::vector. (remote_state::~remote_state) (remote_target::stop_reply_queue_length): Update. (struct queue_iter_param, remove_child_of_pending_fork) (struct check_pending_event_prevents_wildcard_vcont_callback_data) (check_pending_event_prevents_wildcard_vcont_callback) (remove_stop_reply_for_inferior) (remove_stop_reply_of_remote_state) (remote_notif_remove_once_on_match) (stop_reply_match_ptid_and_ws) (remote_kill_child_of_pending_fork): Remove. (remote_target::remove_new_fork_children) (remote_target::check_pending_events_prevent_wildcard_vcont) (remote_target::discard_pending_stop_replies) (remote_target::discard_pending_stop_replies_in_queue) (remote_target::remote_notif_remove_queued_reply) (remote_target::queued_stop_reply) (remote_target::push_stop_reply, remote_target::peek_stop_reply) (remote_target::wait, remote_target::kill_new_fork_children) (remote_target::async): Update. --- gdb/ChangeLog | 24 +++++ gdb/remote.c | 310 ++++++++++++++++------------------------------------------ 2 files changed, 107 insertions(+), 227 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 3013fbe5e6a..c63379fcd25 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -96,10 +96,17 @@ struct protocol_feature; struct packet_reg; struct stop_reply; -typedef struct stop_reply *stop_reply_p; +static void stop_reply_xfree (struct stop_reply *); -DECLARE_QUEUE_P (stop_reply_p); -DEFINE_QUEUE_P (stop_reply_p); +struct stop_reply_deleter +{ + void operator() (stop_reply *r) const + { + stop_reply_xfree (r); + } +}; + +typedef std::unique_ptr stop_reply_up; /* Generic configuration support for packets the stub optionally supports. Allows the user to specify the use of the packet as well @@ -368,7 +375,7 @@ public: /* data */ should be queued first, and be consumed by remote_wait_{ns,as} one per time. Other notifications can consume their events immediately, so queue is not needed for them. */ - QUEUE (stop_reply_p) *stop_reply_queue; + std::vector stop_reply_queue; /* Asynchronous signal handle registered as event loop source for when we have pending events ready to be passed to the core. */ @@ -1021,9 +1028,6 @@ static void show_remote_protocol_packet_cmd (struct ui_file *file, static ptid_t read_ptid (const char *buf, const char **obuf); -struct stop_reply; -static void stop_reply_xfree (struct stop_reply *); - static void remote_async_inferior_event_handler (gdb_client_data); static int remote_read_description_p (struct target_ops *target); @@ -1095,8 +1099,6 @@ remote_state::remote_state () whenever a larger buffer is needed. */ this->buf_size = 400; this->buf = (char *) xmalloc (this->buf_size); - - this->stop_reply_queue = QUEUE_alloc (stop_reply_p, stop_reply_xfree); } remote_state::~remote_state () @@ -1106,7 +1108,6 @@ remote_state::~remote_state () xfree (this->buf); xfree (this->finished_object); xfree (this->finished_annex); - QUEUE_free (stop_reply_p, this->stop_reply_queue); } /* Utility: generate error from an incoming stub packet. */ @@ -6846,7 +6847,7 @@ int remote_target::stop_reply_queue_length () { remote_state *rs = get_remote_state (); - return QUEUE_length (stop_reply_p, rs->stop_reply_queue); + return rs->stop_reply_queue.size (); } void @@ -6929,15 +6930,6 @@ struct notif_client notif_client_stop = REMOTE_NOTIF_STOP, }; -/* A parameter to pass data in and out. */ - -struct queue_iter_param -{ - remote_target *remote; - void *input; - struct stop_reply *output; -}; - /* Determine if THREAD_PTID is a pending fork parent thread. ARG contains the pid of the process that owns the threads we want to check, or -1 if we want to check all threads. */ @@ -6979,27 +6971,6 @@ is_pending_fork_parent_thread (struct thread_info *thread) return is_pending_fork_parent (ws, pid, thread->ptid); } -/* Check whether EVENT is a fork event, and if it is, remove the - fork child from the context list passed in DATA. */ - -static int -remove_child_of_pending_fork (QUEUE (stop_reply_p) *q, - QUEUE_ITER (stop_reply_p) *iter, - stop_reply_p event, - void *data) -{ - struct queue_iter_param *param = (struct queue_iter_param *) data; - struct threads_listing_context *context - = (struct threads_listing_context *) param->input; - - if (event->ws.kind == TARGET_WAITKIND_FORKED - || event->ws.kind == TARGET_WAITKIND_VFORKED - || event->ws.kind == TARGET_WAITKIND_THREAD_EXITED) - context->remove_thread (event->ws.value.related_pid); - - return 1; -} - /* If CONTEXT contains any fork child threads that have not been reported yet, remove them from the CONTEXT list. If such a thread exists it is because we are stopped at a fork catchpoint @@ -7012,7 +6983,6 @@ remote_target::remove_new_fork_children (threads_listing_context *context) struct thread_info * thread; int pid = -1; struct notif_client *notif = ¬if_client_stop; - struct queue_iter_param param; /* For any threads stopped at a fork event, remove the corresponding fork child threads from the CONTEXT list. */ @@ -7028,55 +6998,11 @@ remote_target::remove_new_fork_children (threads_listing_context *context) in process PID and remove those fork child threads from the CONTEXT list as well. */ remote_notif_get_pending_events (notif); - param.remote = this; - param.input = context; - param.output = NULL; - QUEUE_iterate (stop_reply_p, get_remote_state ()->stop_reply_queue, - remove_child_of_pending_fork, ¶m); -} - -/* Callback data for - check_pending_event_prevents_wildcard_vcont_callback. */ -struct check_pending_event_prevents_wildcard_vcont_callback_data -{ - /* The remote target. */ - remote_target *remote; - - /* Whether we can do a global wildcard (vCont;c) */ - int *may_global_wildcard_vcont; -}; - -/* Check whether EVENT would prevent a global or process wildcard - vCont action. */ - -static int -check_pending_event_prevents_wildcard_vcont_callback - (QUEUE (stop_reply_p) *q, - QUEUE_ITER (stop_reply_p) *iter, - stop_reply_p event, - void *data) -{ - struct inferior *inf; - auto *cb_data = (check_pending_event_prevents_wildcard_vcont_callback_data *) data; - - if (event->ws.kind == TARGET_WAITKIND_NO_RESUMED - || event->ws.kind == TARGET_WAITKIND_NO_HISTORY) - return 1; - - if (event->ws.kind == TARGET_WAITKIND_FORKED - || event->ws.kind == TARGET_WAITKIND_VFORKED) - *cb_data->may_global_wildcard_vcont = 0; - - inf = find_inferior_ptid (event->ptid); - - /* This may be the first time we heard about this process. - Regardless, we must not do a global wildcard resume, otherwise - we'd resume this process too. */ - *cb_data->may_global_wildcard_vcont = 0; - if (inf != NULL) - get_remote_inferior (inf)->may_wildcard_vcont = false; - - return 1; + for (auto &event : get_remote_state ()->stop_reply_queue) + if (event->ws.kind == TARGET_WAITKIND_FORKED + || event->ws.kind == TARGET_WAITKIND_VFORKED + || event->ws.kind == TARGET_WAITKIND_THREAD_EXITED) + context->remove_thread (event->ws.value.related_pid); } /* Check whether any event pending in the vStopped queue would prevent @@ -7090,34 +7016,27 @@ remote_target::check_pending_events_prevent_wildcard_vcont (int *may_global_wildcard) { struct notif_client *notif = ¬if_client_stop; - check_pending_event_prevents_wildcard_vcont_callback_data cb_data - {this, may_global_wildcard}; remote_notif_get_pending_events (notif); - QUEUE_iterate (stop_reply_p, get_remote_state ()->stop_reply_queue, - check_pending_event_prevents_wildcard_vcont_callback, - &cb_data); -} + for (auto &event : get_remote_state ()->stop_reply_queue) + { + if (event->ws.kind == TARGET_WAITKIND_NO_RESUMED + || event->ws.kind == TARGET_WAITKIND_NO_HISTORY) + continue; -/* Remove stop replies in the queue if its pid is equal to the given - inferior's pid. */ + if (event->ws.kind == TARGET_WAITKIND_FORKED + || event->ws.kind == TARGET_WAITKIND_VFORKED) + *may_global_wildcard = 0; -static int -remove_stop_reply_for_inferior (QUEUE (stop_reply_p) *q, - QUEUE_ITER (stop_reply_p) *iter, - stop_reply_p event, - void *data) -{ - struct queue_iter_param *param = (struct queue_iter_param *) data; - struct inferior *inf = (struct inferior *) param->input; + struct inferior *inf = find_inferior_ptid (event->ptid); - if (ptid_get_pid (event->ptid) == inf->pid) - { - stop_reply_xfree (event); - QUEUE_remove_elem (stop_reply_p, q, iter); + /* This may be the first time we heard about this process. + Regardless, we must not do a global wildcard resume, otherwise + we'd resume this process too. */ + *may_global_wildcard = 0; + if (inf != NULL) + get_remote_inferior (inf)->may_wildcard_vcont = false; } - - return 1; } /* Discard all pending stop replies of inferior INF. */ @@ -7125,7 +7044,6 @@ remove_stop_reply_for_inferior (QUEUE (stop_reply_p) *q, void remote_target::discard_pending_stop_replies (struct inferior *inf) { - struct queue_iter_param param; struct stop_reply *reply; struct remote_state *rs = get_remote_state (); struct remote_notif_state *rns = rs->notif_state; @@ -7144,34 +7062,15 @@ remote_target::discard_pending_stop_replies (struct inferior *inf) rns->pending_event[notif_client_stop.id] = NULL; } - param.remote = this; - param.input = inf; - param.output = NULL; /* Discard the stop replies we have already pulled with vStopped. */ - QUEUE_iterate (stop_reply_p, rs->stop_reply_queue, - remove_stop_reply_for_inferior, ¶m); -} - -/* If its remote state is equal to the given remote state, - remove EVENT from the stop reply queue. */ - -static int -remove_stop_reply_of_remote_state (QUEUE (stop_reply_p) *q, - QUEUE_ITER (stop_reply_p) *iter, - stop_reply_p event, - void *data) -{ - struct queue_iter_param *param = (struct queue_iter_param *) data; - struct remote_state *rs = (struct remote_state *) param->input; - - if (event->rs == rs) - { - stop_reply_xfree (event); - QUEUE_remove_elem (stop_reply_p, q, iter); - } - - return 1; + auto iter = std::remove_if (rs->stop_reply_queue.begin (), + rs->stop_reply_queue.end (), + [=] (const stop_reply_up &event) + { + return ptid_get_pid (event->ptid) == inf->pid; + }); + rs->stop_reply_queue.erase (iter, rs->stop_reply_queue.end ()); } /* Discard the stop replies for RS in stop_reply_queue. */ @@ -7180,36 +7079,16 @@ void remote_target::discard_pending_stop_replies_in_queue () { remote_state *rs = get_remote_state (); - struct queue_iter_param param; - param.remote = this; - param.input = rs; - param.output = NULL; /* Discard the stop replies we have already pulled with vStopped. */ - QUEUE_iterate (stop_reply_p, rs->stop_reply_queue, - remove_stop_reply_of_remote_state, ¶m); -} - -/* A parameter to pass data in and out. */ - -static int -remote_notif_remove_once_on_match (QUEUE (stop_reply_p) *q, - QUEUE_ITER (stop_reply_p) *iter, - stop_reply_p event, - void *data) -{ - struct queue_iter_param *param = (struct queue_iter_param *) data; - ptid_t *ptid = (ptid_t *) param->input; - - if (ptid_match (event->ptid, *ptid)) - { - param->output = event; - QUEUE_remove_elem (stop_reply_p, q, iter); - return 0; - } - - return 1; + auto iter = std::remove_if (rs->stop_reply_queue.begin (), + rs->stop_reply_queue.end (), + [=] (const stop_reply_up &event) + { + return event->rs == rs; + }); + rs->stop_reply_queue.erase (iter, rs->stop_reply_queue.end ()); } /* Remove the first reply in 'stop_reply_queue' which matches @@ -7218,20 +7097,29 @@ remote_notif_remove_once_on_match (QUEUE (stop_reply_p) *q, struct stop_reply * remote_target::remote_notif_remove_queued_reply (ptid_t ptid) { - struct queue_iter_param param; + remote_state *rs = get_remote_state (); - param.remote = this; - param.input = &ptid; - param.output = NULL; + auto iter = std::find_if (rs->stop_reply_queue.begin (), + rs->stop_reply_queue.end (), + [=] (const stop_reply_up &event) + { + return ptid_match (event->ptid, ptid); + }); + struct stop_reply *result; + if (iter == rs->stop_reply_queue.end ()) + result = nullptr; + else + { + result = iter->release (); + rs->stop_reply_queue.erase (iter); + } - QUEUE_iterate (stop_reply_p, get_remote_state ()->stop_reply_queue, - remote_notif_remove_once_on_match, ¶m); if (notif_debug) fprintf_unfiltered (gdb_stdlog, "notif: discard queued event: 'Stop' in %s\n", target_pid_to_str (ptid)); - return param.output; + return result; } /* Look for a queued stop reply belonging to PTID. If one is found, @@ -7242,11 +7130,11 @@ remote_target::remote_notif_remove_queued_reply (ptid_t ptid) struct stop_reply * remote_target::queued_stop_reply (ptid_t ptid) { + remote_state *rs = get_remote_state (); struct stop_reply *r = remote_notif_remove_queued_reply (ptid); - if (!QUEUE_is_empty (stop_reply_p, get_remote_state ()->stop_reply_queue)) + if (!rs->stop_reply_queue.empty ()) { - remote_state *rs = get_remote_state (); /* There's still at least an event left. */ mark_async_event_handler (rs->remote_async_inferior_event_token); } @@ -7262,38 +7150,28 @@ void remote_target::push_stop_reply (struct stop_reply *new_event) { remote_state *rs = get_remote_state (); - QUEUE_enque (stop_reply_p, rs->stop_reply_queue, new_event); + rs->stop_reply_queue.push_back (stop_reply_up (new_event)); if (notif_debug) fprintf_unfiltered (gdb_stdlog, "notif: push 'Stop' %s to queue %d\n", target_pid_to_str (new_event->ptid), - QUEUE_length (stop_reply_p, - rs->stop_reply_queue)); + int (rs->stop_reply_queue.size ())); mark_async_event_handler (rs->remote_async_inferior_event_token); } -static int -stop_reply_match_ptid_and_ws (QUEUE (stop_reply_p) *q, - QUEUE_ITER (stop_reply_p) *iter, - struct stop_reply *event, - void *data) -{ - ptid_t *ptid = (ptid_t *) data; - - return !(ptid_equal (*ptid, event->ptid) - && event->ws.kind == TARGET_WAITKIND_STOPPED); -} - /* Returns true if we have a stop reply for PTID. */ int remote_target::peek_stop_reply (ptid_t ptid) { remote_state *rs = get_remote_state (); - return !QUEUE_iterate (stop_reply_p, rs->stop_reply_queue, - stop_reply_match_ptid_and_ws, &ptid); + for (auto &event : rs->stop_reply_queue) + if (ptid_equal (ptid, event->ptid) + && event->ws.kind == TARGET_WAITKIND_STOPPED) + return 1; + return 0; } /* Helper for remote_parse_stop_reply. Return nonzero if the substring @@ -8030,7 +7908,7 @@ remote_target::wait (ptid_t ptid, struct target_waitstatus *status, int options) /* If there are are events left in the queue tell the event loop to return here. */ - if (!QUEUE_is_empty (stop_reply_p, rs->stop_reply_queue)) + if (!rs->stop_reply_queue.empty ()) mark_async_event_handler (rs->remote_async_inferior_event_token); } @@ -9793,32 +9671,6 @@ remote_target::getpkt_or_notif_sane (char **buf, long *sizeof_buf, int forever, is_notif); } -/* Check whether EVENT is a fork event for the process specified - by the pid passed in DATA, and if it is, kill the fork child. */ - -int -remote_kill_child_of_pending_fork (QUEUE (stop_reply_p) *q, - QUEUE_ITER (stop_reply_p) *iter, - stop_reply_p event, - void *data) -{ - struct queue_iter_param *param = (struct queue_iter_param *) data; - int parent_pid = *(int *) param->input; - - if (is_pending_fork_parent (&event->ws, parent_pid, event->ptid)) - { - remote_target *remote = param->remote; - int child_pid = ptid_get_pid (event->ws.value.related_pid); - int res; - - res = remote->remote_vkill (child_pid); - if (res != 0) - error (_("Can't kill fork child process %d"), child_pid); - } - - return 1; -} - /* Kill any new fork children of process PID that haven't been processed by follow_fork. */ @@ -9828,7 +9680,6 @@ remote_target::kill_new_fork_children (int pid) remote_state *rs = get_remote_state (); struct thread_info *thread; struct notif_client *notif = ¬if_client_stop; - struct queue_iter_param param; /* Kill the fork child threads of any threads in process PID that are stopped at a fork event. */ @@ -9850,11 +9701,16 @@ remote_target::kill_new_fork_children (int pid) /* Check for any pending fork events (not reported or processed yet) in process PID and kill those fork child threads as well. */ remote_notif_get_pending_events (notif); - param.remote = this; - param.input = &pid; - param.output = NULL; - QUEUE_iterate (stop_reply_p, rs->stop_reply_queue, - remote_kill_child_of_pending_fork, ¶m); + for (auto &event : rs->stop_reply_queue) + if (is_pending_fork_parent (&event->ws, pid, event->ptid)) + { + int child_pid = ptid_get_pid (event->ws.value.related_pid); + int res; + + res = remote_vkill (child_pid); + if (res != 0) + error (_("Can't kill fork child process %d"), child_pid); + } } @@ -14188,7 +14044,7 @@ remote_target::async (int enable) /* If there are pending events in the stop reply queue tell the event loop to process them. */ - if (!QUEUE_is_empty (stop_reply_p, rs->stop_reply_queue)) + if (!rs->stop_reply_queue.empty ()) mark_async_event_handler (rs->remote_async_inferior_event_token); /* For simplicity, below we clear the pending events token without remembering whether it is marked, so here we always