From patchwork Mon Oct 16 22:34:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 23629 Received: (qmail 116498 invoked by alias); 16 Oct 2017 22:35:04 -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 116443 invoked by uid 89); 16 Oct 2017 22:35:04 -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= 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; Mon, 16 Oct 2017 22:35:02 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 66E2F37E6E; Mon, 16 Oct 2017 22:35:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 66E2F37E6E 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@redhat.com 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 2214F5D6A9; Mon, 16 Oct 2017 22:34:59 +0000 (UTC) Subject: Re: [RFA 4/6] Simple cleanup removals in remote.c To: Simon Marchi , Tom Tromey References: <20171016030427.21349-1-tom@tromey.com> <20171016030427.21349-5-tom@tromey.com> <07804bc3-a6c5-2c0f-5730-5dd12fccafbe@ericsson.com> <87fuaipzgg.fsf@tromey.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <97a40149-a30f-b2af-4441-6945b1d29cf1@redhat.com> Date: Mon, 16 Oct 2017 23:34:59 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: On 10/16/2017 10:36 PM, Simon Marchi wrote: > On 2017-10-16 05:14 PM, Tom Tromey wrote: >>>>>>> "Simon" == Simon Marchi writes: >> >> Simon> For this one, wouldn't it be easier to just go with a string? Something like: >> >> It's easier but less efficient. >> I wasn't sure if it mattered so I erred on the side of efficiency. >> >> Tom >> > > I suppose you are talking about the += string_printf ()? It's true that it's > not the most efficient, because the formatting is done in a separate buffer > (an std::string) and then copied, instead of directly in-place. At least, > there's likely no dynamic memory allocation, because the string fits in > std::string's local buffer. This suggests to me that we're missing a string_printf variant that appends to a preexisting string: void string_appendf (std::string &dest, const char* fmt, ...); See (untested) patch below. > > So since there's likely no real-world impact, I would've erred on the other > side. > > In any case, I don't really mind, your version is good too and eliminates the > clean up, which is the main point. I agree. Tom, please go ahead. From e9602dc6ee8eac1f0dc5b267e961f9a206ce649b Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Mon, 16 Oct 2017 23:22:27 +0100 Subject: [PATCH] string_appendf --- gdb/common/common-utils.c | 22 ++++++++++++++++++++++ gdb/common/common-utils.h | 5 +++++ gdb/remote.c | 22 +++++++--------------- 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c index d8c546a..8ca62f3 100644 --- a/gdb/common/common-utils.c +++ b/gdb/common/common-utils.c @@ -153,6 +153,28 @@ xsnprintf (char *str, size_t size, const char *format, ...) /* See documentation in common-utils.h. */ +void +string_appendf (std::string &str, const char* fmt, ...) +{ + va_list vp; + int grow_size; + + va_start (vp, fmt); + grow_size = vsnprintf (NULL, 0, fmt, vp); + va_end (vp); + + size_t curr_size = str.size (); + str.resize (curr_size + grow_size); + + /* C++11 and later guarantee std::string uses contiguous memory and + always includes the terminating '\0'. */ + va_start (vp, fmt); + vsprintf (&str[curr_size], fmt, vp); + va_end (vp); +} + +/* See documentation in common-utils.h. */ + std::string string_printf (const char* fmt, ...) { diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h index 19724f9..bf1b444 100644 --- a/gdb/common/common-utils.h +++ b/gdb/common/common-utils.h @@ -67,6 +67,11 @@ std::string string_printf (const char* fmt, ...) std::string string_vprintf (const char* fmt, va_list args) ATTRIBUTE_PRINTF (1, 0); +/* Like string_printf, but appends to dest instead of returning a new + std::string. */ +void string_appendf (std::string &dest, const char* fmt, ...) + ATTRIBUTE_PRINTF (2, 3); + /* Make a copy of the string at PTR with LEN characters (and add a null character at the end in the copy). Uses malloc to get the space. Returns the address of the copy. */ diff --git a/gdb/remote.c b/gdb/remote.c index 6b77a9f..a6cb724 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -2086,40 +2086,32 @@ remote_set_syscall_catchpoint (struct target_ops *self, pid, needed, any_count, n_sysno); } - gdb::unique_xmalloc_ptr built_packet; + std::string built_packet; if (needed) { /* Prepare a packet with the sysno list, assuming max 8+1 characters for a sysno. If the resulting packet size is too big, fallback on the non-selective packet. */ const int maxpktsz = strlen ("QCatchSyscalls:1") + n_sysno * 9 + 1; - - built_packet.reset ((char *) xmalloc (maxpktsz)); - strcpy (built_packet.get (), "QCatchSyscalls:1"); + built_packet.reserve (maxpktsz); + built_packet = "QCatchSyscalls:1"; if (!any_count) { - int i; - char *p; - - p = built_packet.get (); - p += strlen (p); - /* Add in catch_packet each syscall to be caught (table[i] != 0). */ - for (i = 0; i < table_size; i++) + for (int i = 0; i < table_size; i++) { if (table[i] != 0) - p += xsnprintf (p, built_packet.get () + maxpktsz - p, - ";%x", i); + string_appendf (built_packet, ";%x", i); } } - if (strlen (built_packet.get ()) > get_remote_packet_size ()) + if (built_packet.size () > get_remote_packet_size ()) { /* catch_packet too big. Fallback to less efficient non selective mode, with GDB doing the filtering. */ catch_packet = "QCatchSyscalls:1"; } else - catch_packet = built_packet.get (); + catch_packet = built_packet.c_str (); } else catch_packet = "QCatchSyscalls:0";