From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 108957 invoked by alias); 6 Mar 2018 07:30:50 -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 108836 invoked by uid 89); 6 Mar 2018 07:30:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS,TIME_LIMIT_EXCEEDED,T_RP_MATCHES_RCVD autolearn=unavailable version=3.3.2 spammy=Hx-languages-length:2294 X-HELO: mga14.intel.com Received: from mga14.intel.com (HELO mga14.intel.com) (192.55.52.115) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 06 Mar 2018 07:30:13 +0000 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Mar 2018 23:30:12 -0800 X-ExtLoop1: 1 Received: from irsmsx107.ger.corp.intel.com ([163.33.3.99]) by orsmga008.jf.intel.com with ESMTP; 05 Mar 2018 23:30:11 -0800 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.101]) by IRSMSX107.ger.corp.intel.com ([169.254.10.46]) with mapi id 14.03.0319.002; Tue, 6 Mar 2018 07:30:10 +0000 From: "Metzger, Markus T" To: Simon Marchi CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH 3/3] btrace: Remove ui_out cleanups Date: Tue, 06 Mar 2018 07:30:00 -0000 Message-ID: References: <20180304205605.13037-1-simon.marchi@polymtl.ca> <20180304205605.13037-3-simon.marchi@polymtl.ca> In-Reply-To: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDI4MzAyOWItYjE5Yy00YjY5LWFmODItNWVkNDRlMGZkNzM3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJhczg3YXVOVmtOcmpsTGR1cllHK0FHTVR6R1RHeEdSdXh4dEJMQ1Z6UUdOcGV2cGRrcjY2WWR2R2s1M21DdjFVIn0= dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2018-03/txt/msg00130.txt.bz2 Hello Simon, > >> static void > >> btrace_print_lines (struct btrace_line_range lines, struct ui_out > >> *uiout, > >> - struct cleanup **ui_item_chain, int flags) > >> + gdb::optional *src_and_asm_tuple, > >> + gdb::optional *asm_list, > > > > Reference instead of pointer? >=20 > I once pointed this out on one of Tom's patches, and he said that in the > caller code, it's more obvious that object is meant to be modified if > you do: >=20 > function_that_modifies_object (&object); >=20 > instead of >=20 > function_that_modifies_object (object); >=20 > And I kind of agree with that it is, which is why I've been using > pointers when references would have worked (ok, here the function name > makes it obvious but it's not always the case). In the implementation > of the function, since you use object->field instead of object.field, it > also hints that you're not modifying a local object. But I also agree > that it's really not C++-y to do it this way, so I'll happily change it. I prefer consistency. I we agreed to use pointers instead of references in other parts of GDB, let's do so everywhere. > Yes, I have ran the gdb.btrace/*.exp tests locally on two different > machines and saw no regressions. However, the processors may be a bit > old (Q6600 from 2007 and i5-4310U from 2014), so it's possible that not > all required features are available, and therefore some tests may be > skipped. So if you want to be sure, here's a branch for you to test: You would get an "untested" if btrace tests are skipped. As long as you're not getting all "untested", you should be fine. There is only one test, tsx.exp, that requires recent hardware and compiler. It would use the method that is available on your target preferring PT over BTS. But this change is not related to trace decode so it shouldn't matter. I ran the tests on recent hardware using PT and everything passes. Thanks, Markus. Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Christian Lamprechter Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928