From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32771 invoked by alias); 18 Sep 2017 16:34:53 -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 32761 invoked by uid 89); 18 Sep 2017 16:34:53 -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_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mga07.intel.com Received: from mga07.intel.com (HELO mga07.intel.com) (134.134.136.100) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 18 Sep 2017 16:34:51 +0000 Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP; 18 Sep 2017 09:34:49 -0700 X-ExtLoop1: 1 Received: from labpc1530.iul.intel.com ([172.28.48.195]) by fmsmga004.fm.intel.com with ESMTP; 18 Sep 2017 09:34:47 -0700 Subject: Re: [PATCH] icc: allow code path for newer versions of icc. To: Simon Marchi , Pedro Alves References: <1504687613-14649-1-git-send-email-walfred.tedeschi@intel.com> <59e18301-7b12-2e92-6277-0aef4164243e@redhat.com> Cc: qiyaoltc@gmail.com, gdb-patches@sourceware.org From: "Tedeschi, Walfred" Message-ID: <914ad9ed-940e-81e1-e7de-6c018d29aec7@intel.com> Date: Mon, 18 Sep 2017 16:34: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: Content-Type: text/plain; charset="windows-1252"; format="flowed" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00450.txt.bz2 Hi Pedro and Simon, Thanks again for the review! On 09/18/2017 06:23 PM, Simon Marchi wrote: > On 2017-09-18 17:34, Pedro Alves wrote: >> Hi, >> >> I read the patch and I found a few nits to pick. >> See below. >> >> On 09/06/2017 09:46 AM, Walfred Tedeschi wrote: >>> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c >>> index 6678b33..4aa3b3e 100644 >>> --- a/gdb/dwarf2read.c >>> +++ b/gdb/dwarf2read.c >>> @@ -604,7 +604,7 @@ struct dwarf2_cu >>> unsigned int checked_producer : 1; >> >> ... >> >>> >>> +#if defined GDB_SELF_TEST >>> +#include "selftest.h" >>> + >>> +namespace selftests { >>> +namespace gdbserver { >>> +static void >>> +dwarf_producer_test () >> >> Simon already pointed at the gdbserver namespace. I'd like >> to add that it looks odd to me to define the actual >> functionality in utils.c and define the corresponding self >> tests in dwarf2read.c. Why not put the tests in utils.c as well? >> >> BTW, I'd support moving all these producer checks to >> a separate file (maybe producer.c), instead of putting >> it all in the utils.c kitchen sync, though that's a >> preexisting issue. > > I agree about that. I almost wrote that this function didn't really=20 > belong in utils.c, until I saw that it's where other similar functions=20 > were already. > > Simon I am considering sending then two patches: 1. Add things in utils.c 2. move all producers to a new producers.h/c file. Would this be ok? Thanks again, /Fred 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