From patchwork Tue May 22 17:38:12 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 27405 Received: (qmail 52158 invoked by alias); 22 May 2018 17:38:17 -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 52148 invoked by uid 89); 22 May 2018 17:38:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 22 May 2018 17:38:15 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D8A09818BAFF; Tue, 22 May 2018 17:38:13 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4D4412024CAD; Tue, 22 May 2018 17:38:13 +0000 (UTC) Subject: Re: [PATCH 09/10] remote: Make vcont_builder a class To: Simon Marchi , gdb-patches@sourceware.org References: <20180516141830.16859-1-palves@redhat.com> <20180516141830.16859-10-palves@redhat.com> <603566f0-e2f8-236c-7593-ea86943d49f0@simark.ca> From: Pedro Alves Message-ID: Date: Tue, 22 May 2018 18:38:12 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <603566f0-e2f8-236c-7593-ea86943d49f0@simark.ca> On 05/22/2018 04:06 AM, Simon Marchi wrote: > On 2018-05-16 10:18 AM, Pedro Alves wrote: >> gdb/ChangeLog: >> yyyy-mm-dd Pedro Alves >> >> * remote.c (vcont_builder): Now a class. Make all data members >> private. >> (vcont_builder) : >> Declare methods. >> (vcont_builder_restart): Rename to ... >> (vcont_builder::restart): ... this. >> (vcont_builder_flush): Rename to ... >> (vcont_builder::flush): ... this. >> (vcont_builder_push_action): Rename to ... >> (vcont_builder::push_action): ... this. >> (remote_target::commit_resume): Adjust. > > LGTM. Here's the version that I'm landing -- it moves the restart() call to the ctor, as suggested in response to patch #10. From f5db4863f51b83bc6b54504d61e1731011cfdec2 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 22 May 2018 18:22:11 +0100 Subject: [PATCH] remote: Make vcont_builder a class gdb/ChangeLog: 2018-05-22 Pedro Alves * remote.c (vcont_builder): Now a class. Make all data members private. (vcont_builder) : Declare methods. (vcont_builder_restart): Rename to ... (vcont_builder::restart): ... this. (vcont_builder_flush): Rename to ... (vcont_builder::flush): ... this. (vcont_builder_push_action): Rename to ... (vcont_builder::push_action): ... this. (remote_target::commit_resume): Adjust. --- gdb/ChangeLog | 14 ++++++++++ gdb/remote.c | 83 ++++++++++++++++++++++++++++++++--------------------------- 2 files changed, 59 insertions(+), 38 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 3504a3cf19..501ee86a2a 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,17 @@ +2018-05-22 Pedro Alves + + * remote.c (vcont_builder): Now a class. Make all data members + private. + (vcont_builder) : + Declare methods. + (vcont_builder_restart): Rename to ... + (vcont_builder::restart): ... this. + (vcont_builder_flush): Rename to ... + (vcont_builder::flush): ... this. + (vcont_builder_push_action): Rename to ... + (vcont_builder::push_action): ... this. + (remote_target::commit_resume): Adjust. + 2018-05-22 Pedro Alves * remote.c (DEFAULT_MAX_MEMORY_PACKET_SIZE): Rename to ... diff --git a/gdb/remote.c b/gdb/remote.c index 72254dba31..33b2c906eb 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -6096,45 +6096,57 @@ get_remote_inferior (inferior *inf) return static_cast (inf->priv.get ()); } -/* Structure used to track the construction of a vCont packet in the +/* Class used to track the construction of a vCont packet in the outgoing packet buffer. This is used to send multiple vCont packets if we have more actions than would fit a single packet. */ -struct vcont_builder +class vcont_builder { +public: + vcont_builder () + { + restart (); + } + + void flush (); + void push_action (ptid_t ptid, bool step, gdb_signal siggnal); + +private: + void restart (); + /* Pointer to the first action. P points here if no action has been appended yet. */ - char *first_action; + char *m_first_action; /* Where the next action will be appended. */ - char *p; + char *m_p; /* The end of the buffer. Must never write past this. */ - char *endp; + char *m_endp; }; /* Prepare the outgoing buffer for a new vCont packet. */ -static void -vcont_builder_restart (struct vcont_builder *builder) +void +vcont_builder::restart () { struct remote_state *rs = get_remote_state (); - builder->p = rs->buf; - builder->endp = rs->buf + get_remote_packet_size (); - builder->p += xsnprintf (builder->p, builder->endp - builder->p, "vCont"); - builder->first_action = builder->p; + m_p = rs->buf; + m_endp = rs->buf + get_remote_packet_size (); + m_p += xsnprintf (m_p, m_endp - m_p, "vCont"); + m_first_action = m_p; } /* If the vCont packet being built has any action, send it to the remote end. */ -static void -vcont_builder_flush (struct vcont_builder *builder) +void +vcont_builder::flush () { struct remote_state *rs; - if (builder->p == builder->first_action) + if (m_p == m_first_action) return; rs = get_remote_state (); @@ -6155,33 +6167,30 @@ vcont_builder_flush (struct vcont_builder *builder) what we've got so far to the remote end and start over a new vCont packet (with the new action). */ -static void -vcont_builder_push_action (struct vcont_builder *builder, - ptid_t ptid, int step, enum gdb_signal siggnal) +void +vcont_builder::push_action (ptid_t ptid, bool step, gdb_signal siggnal) { char buf[MAX_ACTION_SIZE + 1]; - char *endp; - size_t rsize; - endp = append_resumption (buf, buf + sizeof (buf), - ptid, step, siggnal); + char *endp = append_resumption (buf, buf + sizeof (buf), + ptid, step, siggnal); /* Check whether this new action would fit in the vCont packet along with previous actions. If not, send what we've got so far and start a new vCont packet. */ - rsize = endp - buf; - if (rsize > builder->endp - builder->p) + size_t rsize = endp - buf; + if (rsize > m_endp - m_p) { - vcont_builder_flush (builder); - vcont_builder_restart (builder); + flush (); + restart (); /* Should now fit. */ - gdb_assert (rsize <= builder->endp - builder->p); + gdb_assert (rsize <= m_endp - m_p); } - memcpy (builder->p, buf, rsize); - builder->p += rsize; - *builder->p = '\0'; + memcpy (m_p, buf, rsize); + m_p += rsize; + *m_p = '\0'; } /* to_commit_resume implementation. */ @@ -6193,7 +6202,6 @@ remote_target::commit_resume () struct thread_info *tp; int any_process_wildcard; int may_global_wildcard_vcont; - struct vcont_builder vcont_builder; /* If connected in all-stop mode, we'd send the remote resume request directly from remote_resume. Likewise if @@ -6291,7 +6299,7 @@ remote_target::commit_resume () we end up with too many actions for a single packet vcont_builder flushes the current vCont packet to the remote side and starts a new one. */ - vcont_builder_restart (&vcont_builder); + struct vcont_builder vcont_builder; /* Threads first. */ ALL_NON_EXITED_THREADS (tp) @@ -6312,7 +6320,7 @@ remote_target::commit_resume () continue; } - vcont_builder_push_action (&vcont_builder, tp->ptid, + vcont_builder.push_action (tp->ptid, remote_thr->last_resume_step, remote_thr->last_resume_sig); remote_thr->vcont_resumed = 1; @@ -6339,8 +6347,8 @@ remote_target::commit_resume () continue action for each running process, if any. */ if (may_global_wildcard_vcont) { - vcont_builder_push_action (&vcont_builder, minus_one_ptid, - 0, GDB_SIGNAL_0); + vcont_builder.push_action (minus_one_ptid, + false, GDB_SIGNAL_0); } else { @@ -6348,15 +6356,14 @@ remote_target::commit_resume () { if (get_remote_inferior (inf)->may_wildcard_vcont) { - vcont_builder_push_action (&vcont_builder, - pid_to_ptid (inf->pid), - 0, GDB_SIGNAL_0); + vcont_builder.push_action (pid_to_ptid (inf->pid), + false, GDB_SIGNAL_0); } } } } - vcont_builder_flush (&vcont_builder); + vcont_builder.flush (); }