From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32962 invoked by alias); 12 Jan 2017 15:09:06 -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 32951 invoked by uid 89); 12 Jan 2017 15:09:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 spammy=amid, Hx-languages-length:2423, feelings, unreadable X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 12 Jan 2017 15:09:02 +0000 Received: from pps.filterd (m0098393.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id v0CF5Ia6050191 for ; Thu, 12 Jan 2017 10:09:01 -0500 Received: from e06smtp06.uk.ibm.com (e06smtp06.uk.ibm.com [195.75.94.102]) by mx0a-001b2d01.pphosted.com with ESMTP id 27x9h8f47p-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 12 Jan 2017 10:09:01 -0500 Received: from localhost by e06smtp06.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 12 Jan 2017 15:08:58 -0000 Received: from d06dlp02.portsmouth.uk.ibm.com (9.149.20.14) by e06smtp06.uk.ibm.com (192.168.101.136) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 12 Jan 2017 15:08:56 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id E41BD219005F; Thu, 12 Jan 2017 15:08:02 +0000 (GMT) Received: from d06av21.portsmouth.uk.ibm.com (d06av21.portsmouth.uk.ibm.com [9.149.105.232]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v0CF8tBw20054522; Thu, 12 Jan 2017 15:08:55 GMT Received: from d06av21.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0DD1E5204B; Thu, 12 Jan 2017 14:07:25 +0000 (GMT) Received: from ThinkPad (unknown [9.152.212.192]) by d06av21.portsmouth.uk.ibm.com (Postfix) with ESMTP id E1D085204F; Thu, 12 Jan 2017 14:07:24 +0000 (GMT) Date: Thu, 12 Jan 2017 15:09:00 -0000 From: Philipp Rudo To: Pedro Alves Cc: gdb-patches@sourceware.org, peter.griffin@linaro.org, yao.qi@arm.com, arnez@linux.vnet.ibm.com Subject: Re: [RFC 2/7] Add libiberty/concat styled concat_path function In-Reply-To: <38e9621a-20a3-730a-b5bb-6fdcb5cea6b4@redhat.com> References: <20170112113217.48852-1-prudo@linux.vnet.ibm.com> <20170112113217.48852-3-prudo@linux.vnet.ibm.com> <20170112143315.338f6cfe@ThinkPad> <38e9621a-20a3-730a-b5bb-6fdcb5cea6b4@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17011215-0024-0000-0000-000002845862 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17011215-0025-0000-0000-000021EC62FF Message-Id: <20170112160854.0040376a@ThinkPad> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-01-12_11:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1701120210 X-IsSubscribed: yes X-SW-Source: 2017-01/txt/msg00229.txt.bz2 On Thu, 12 Jan 2017 13:48:03 +0000 Pedro Alves wrote: > 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). You are right here. > 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. Thanks for the tip. I will check my patches again. > I hope it's obvious that I'm all for C++ conversion, but ... its quite obvious that you are pro C++. My own feelings are quite mixed. I must amid that the standard library is quite handy, at least when it works. Debugging it is quite a pain. But the syntax sometimes is unreadable, especially when you use those teplates containing templates using types is different namespaces. Furthermore i don't think that simply porting GDB to C++ solves its major problem that there are no clear structures in code. Although it could help if you not only port it one to one but restructure the code while you work at it. But that would requires quite some work... > > 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. As much as i know about C++ this sounds like a good idea. Thanks Philipp