From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16806 invoked by alias); 10 Oct 2014 06:58:45 -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 16794 invoked by uid 89); 10 Oct 2014 06:58:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mga01.intel.com Received: from mga01.intel.com (HELO mga01.intel.com) (192.55.52.88) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 10 Oct 2014 06:58:38 +0000 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 09 Oct 2014 23:57:54 -0700 X-ExtLoop1: 1 Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by fmsmga002.fm.intel.com with ESMTP; 09 Oct 2014 23:57:43 -0700 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.248]) by IRSMSX101.ger.corp.intel.com ([169.254.1.201]) with mapi id 14.03.0195.001; Fri, 10 Oct 2014 07:57:43 +0100 From: "Tedeschi, Walfred" To: Doug Evans CC: "palves@redhat.com" , "mark.kettenis@xs4all.nl" , "gdb-patches@sourceware.org" Subject: RE: [PATCH, PR 17364] Need better printer names in bound_register.py Date: Fri, 10 Oct 2014 06:58:00 -0000 Message-ID: References: <1412062133-22469-1-git-send-email-walfred.tedeschi@intel.com> <21558.50988.529374.214950@ruffy2.mtv.corp.google.com> In-Reply-To: <21558.50988.529374.214950@ruffy2.mtv.corp.google.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2014-10/txt/msg00231.txt.bz2 Hello Doug, Thanks a lot for the nice and detailed explanation. I am fine to stick with= the patch you mentioned. Grouping the pretty-printers is indeed a nice idea! Sorry for having taking some time to answer to the defect. I usually try to= answer quick! :( Do you think we should after that think about a mechanism to add this print= er according to the architecture? Thanks again and best regards, -Fred -----Original Message----- From: Doug Evans [mailto:dje@google.com]=20 Sent: Thursday, October 09, 2014 7:35 PM To: Tedeschi, Walfred Cc: palves@redhat.com; mark.kettenis@xs4all.nl; gdb-patches@sourceware.org Subject: Re: [PATCH, PR 17364] Need better printer names in bound_register.= py Walfred Tedeschi writes: > Moved the printer to the global scope and changed the name of the > pri= nter to a more specific name. > > 2014.09.11 Walfred Tedeschi > > python/= lib/gdb/command: >=20 > * bound_registers.py (mpx_bound_reg_printer) Added function to > register pretty-printing for bound registers, and fixed comments. > (build_pretty_printer) Removed. Hi. fwiw, I kinda like grouping together all the pretty-printers provided by gd= b. See https://sourceware.org/ml/gdb-patches/2014-10/msg00081.html Using RegexpCollectionPrettyPrinter is a bit of overkill since there are no= templates, at least not yet. We could certainly add SimpleCollectionPrett= yPrinter or some such that just did string comparisons on tag names instead= of regexps, but since this is all implementation detail we could defer thi= s. > +def mpx_bound_reg_printer (val): > + lookup_tag =3D val.type.tag > + if lookup_tag =3D=3D None: > + return None > + if lookup_tag =3D=3D "__gdb_builtin_type_bound128": > + return BoundPrinter (val) Random comments: 1) "lookup_tag" is a bit confusing as there is no lookup done here. I'd rename it to just val_type_tag or some such. [Assuming we keep this version.] 2) "mpx" is presumably enough to distinguish this pretty-printer from other= arch's bounds reg pretty-printers (probably a better choice than "x86" too= ). 3) If I do "info pretty" I see this: global pretty-printers: mpx_bound_reg_printer Having "printer" in the name is superfluous, thus if one was to do things t= his way I'd rename mpx_bound_reg_printer to mpx_bound_reg. That way the user can do "disable pretty-printer global mpx_bound_reg". 4) It might be preferable to pick a name closer to the actual type's name: "mpx_bound128" ? [digression: Registering printers as functions was how pretty-printers were originally a= dded, but it turned out to be insufficient: printers need to be disableable= and thus need names. OTOH one tends to think of the name of the function = here as an implementation detail, except that it isn't.] --- Going forward, I like the idea of providing basic infrastructure for grouping builtin pret= ty-printers together and using that for 17364. Plus this file really doesn't belong in python/lib/gdb/command. I propose sticking with this patch https://sourceware.org/ml/gdb-patches/2014-10/msg00081.html but I'm happy to tweak it as desired (e.g. use "mpx_bound128" as the name). Intel GmbH Dornacher Strasse 1 85622 Feldkirchen/Muenchen, Deutschland Sitz der Gesellschaft: Feldkirchen bei Muenchen Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk Registergericht: Muenchen HRB 47456 Ust.-IdNr./VAT Registration No.: DE129385895 Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052