From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14671 invoked by alias); 28 Jan 2015 14:32: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 14618 invoked by uid 89); 28 Jan 2015 14:32:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,SPF_PASS,T_RP_MATCHES_RCVD 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; Wed, 28 Jan 2015 14:32:03 +0000 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 28 Jan 2015 06:31:55 -0800 X-ExtLoop1: 1 Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by fmsmga002.fm.intel.com with ESMTP; 28 Jan 2015 06:31:54 -0800 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.229]) by IRSMSX103.ger.corp.intel.com ([169.254.3.242]) with mapi id 14.03.0195.001; Wed, 28 Jan 2015 14:31:53 +0000 From: "Metzger, Markus T" To: Pedro Alves CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH v2 08/13] btrace: move and rename btrace-common Date: Wed, 28 Jan 2015 17:58:00 -0000 Message-ID: References: <1416480444-9943-1-git-send-email-markus.t.metzger@intel.com> <1416480444-9943-9-git-send-email-markus.t.metzger@intel.com> <54C7739C.1050701@redhat.com> In-Reply-To: <54C7739C.1050701@redhat.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-01/txt/msg00734.txt.bz2 > -----Original Message----- > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] On Behalf Of Pedro Alves > Sent: Tuesday, January 27, 2015 12:17 PM > To: Metzger, Markus T > Cc: gdb-patches@sourceware.org > Subject: Re: [PATCH v2 08/13] btrace: move and rename btrace-common >=20 > On 11/20/2014 10:47 AM, Markus Metzger wrote: > > diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c > > index ebacd96..4ed9837 100644 > > --- a/gdb/gdbserver/server.c > > +++ b/gdb/gdbserver/server.c > > @@ -30,7 +30,7 @@ > > #endif > > #include "gdb_vecs.h" > > #include "gdb_wait.h" > > -#include "btrace-common.h" > > +#include "nat/x86-btrace.h" > > #include "filestuff.h" > > #include "tracepoint.h" > > #include "dll.h" >=20 > This still looks like layering violation this way. > server.c should not really have x86 specific bits either. >=20 > My original comment on v1 was: >=20 > >> diff --git a/gdb/common/btrace-common.c b/gdb/common/btrace- > common.c > >> > index 90774a2..178ad35 100644 > >> > --- a/gdb/common/btrace-common.c > >> > +++ b/gdb/common/btrace-common.c > >> > @@ -18,10 +18,47 @@ > >> > along with this program. If not, see . > */ > >> > > >> > #include "btrace-common.h" > >> > +#include "nat/i386-cpuid.h" > > Hm, this is a layering violation. > > common/ files must not include nat/ files. > > If btrace-common.c is only used by native code, then it should > > itself be in nat/ too. >=20 > So, what does btrace-common.h actually contain that server.c needs? > "common" as a moniker doesn't really help much, as it doesn't > really indicate what the code is about. Is it just common > definitions that target-independent parts of gdb and/or gdbserver > use? How about something like leaving those parts in common/ (bonus > points of naming the file something more indicative or what is > contains), and then the native bits, like the x86 probing would > be put under nat/x86-btrace.h|c. What introduced the x86 dependency was the new function btrace_this_cpu, which needs x86-cpuid.h. I put the declaration of struct btrace_cpu in btrace-common.h because it will be needed for struct btrace_data_pt_config, which will be needed by struct btrace_data. The new function btrace_this_cpu goes into nat/linux-btrace.c where it is used. Now nat/linux-btrace.c has an x86 dependency. Otoh, the entire feature is x86-dependent so we might as well rename it to nat/x86-linux-btrace.c, if you want. I dropped this patch and merged the two cpu identification patches. Regarding a better name, the file contains configuration and data representation stuff. It could be split into btrace-config.h and btrace-data.h. Or we could just leave it as btrace-common.h. regards, markus. 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