From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 85211 invoked by alias); 12 Jan 2017 13:48:31 -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 85069 invoked by uid 89); 12 Jan 2017 13:48:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:1543, SLASH_STRING, slash_string, destroys 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; Thu, 12 Jan 2017 13:48:10 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 76C985A71; Thu, 12 Jan 2017 13:48:10 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0CDm47A010389; Thu, 12 Jan 2017 08:48:05 -0500 Subject: Re: [RFC 2/7] Add libiberty/concat styled concat_path function To: Philipp Rudo References: <20170112113217.48852-1-prudo@linux.vnet.ibm.com> <20170112113217.48852-3-prudo@linux.vnet.ibm.com> <20170112143315.338f6cfe@ThinkPad> Cc: gdb-patches@sourceware.org, peter.griffin@linaro.org, yao.qi@arm.com, arnez@linux.vnet.ibm.com From: Pedro Alves Message-ID: <38e9621a-20a3-730a-b5bb-6fdcb5cea6b4@redhat.com> Date: Thu, 12 Jan 2017 13:48:00 -0000 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: <20170112143315.338f6cfe@ThinkPad> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-01/txt/msg00225.txt.bz2 On 01/12/2017 01:33 PM, Philipp Rudo wrote: > Hi Pedro, > > i see your point. > > My goal here was to get rid of any C-string. While making this patch i > also wanted to use it to get rid of all those > > concat (path, need_dirsep ? SLASH_STRING : "", NULL) > > or > > strcat (path, "/") > strcat (path, file) > > constructs. I gave up when it repeatedly caused memory leaks and use > after free errors because of the mixture of C and C++ strings. Fixing > them made the code less readable than before. Thus you should only use > one kind of string through out GDB, either char * or std::string. And > as GDB decided to move to C++ for me std::string is the way you should > go. Even if we used std::string throughout, we should still be careful with unnecessary string copying. "std::string" vs "const string &" in function parameters (use the former only when the function already needs to work with a copy). Similarly, please don't write: + for (std::string arg: args) + { Please write instead: for (const std::string &arg : args) Or for (const auto &arg : args) "for (std::string arg: args)" creates/destroys one deep string copy on each iteration. I hope it's obvious that I'm all for C++ conversion, but ... > Even when it costs performance. ... not at any cost. startswith _is_ used in performance critical paths. BTW, I've been thinking that we may want to add our version of C++17 std::string_view to avoid these kinds of problems. Thanks, Pedro Alves