From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3902 invoked by alias); 11 Dec 2012 20:07:50 -0000 Received: (qmail 3877 invoked by uid 22791); 11 Dec 2012 20:07:49 -0000 X-SWARE-Spam-Status: No, hits=-7.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 11 Dec 2012 20:07:42 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBBK7XNl031921 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 11 Dec 2012 15:07:34 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qBBK7VYd025370; Tue, 11 Dec 2012 15:07:33 -0500 Message-ID: <50C79283.1050608@redhat.com> Date: Tue, 11 Dec 2012 20:07:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Andreas Tobler CC: gdb-patches@sourceware.org Subject: Re: [PATCH] FreeBSD powerpc support References: <50AAA80B.1000509@fgznet.ch> <50AFCF95.1080809@redhat.com> <50B4FC29.9050006@fgznet.ch> In-Reply-To: <50B4FC29.9050006@fgznet.ch> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2012-12/txt/msg00355.txt.bz2 On 11/27/2012 05:45 PM, Andreas Tobler wrote: >> - If the function implements of a target/gdbarch/etc. method, then >> comment it as such. E.g., >> >> /* This is the implementation of gdbarch method FOOBAR. */ >> >> - I noticed some functions with long comments are copies of existing >> code of other ports. I wonder if we could perhaps share more code. > > True, here I do not know how to share, maybe a common ppc64-tdep-common.c? The tdep files are usually called ARCH-OS-tdep.c, so that would be ppc64-common-tdep.c. That raises the question of what's different between ppc64-common-tdep.c and ppc64-tdep.c. Clearer alternatives could be ppc64-unix-tdep.c or some other token that describes the commonality between what's being shared (as opposed to just the fact that something is shared, as in "common"). In any case, the name of the file is the minor detail. >> read_memory_unsigned_integer nowadays has a byte_order parameter, >> so just pass it BFD_ENDIAN_BIG, and you're set. > > Done. Even tested on ppc64-linux. I might send a patch for > ppc-linux-tdep.c as well. Likewise for the below. That'd be nice. >> For some reason this bailing out if name is not null jumped at me. >> It's not obvious to me what that means, so it felt like it deserves >> a comment. > > Also done, I hope I match the expectations. You have, thanks. > >> On 11/19/2012 09:43 PM, Andreas Tobler wrote: >>> --- configure.host 30 May 2012 19:41:34 -0000 1.107 >>> +++ configure.host 19 Nov 2012 21:24:15 -0000 >>> @@ -125,6 +125,7 @@ >>> >>> powerpc-*-aix* | rs6000-*-*) >>> gdb_host=aix ;; >>> +powerpc*-*-freebsd*) gdb_host=fbsd ;; >> >> This seems to be 'powerpc-*-freebsd*' elsewhere I looked (top level, bfd). >> Why the extra wildcard? > > > Hm, here I'm not sure. My targets report as powerpc-unknown-freebsd* > (32-bit) and powerpc64-unknown-freebsd* (64-bit). So I thought I have to > match both. Is this not correct? I guess that's fine then. > Attached a revised version of the diff. Thanks. Please always paste the ChangeLog entry along with the patch too, even if it hadn't needed changes. > +#include "features/rs6000/powerpc-32l.c" > +#include "features/rs6000/powerpc-64l.c" This is a problem. I notice you have tdesc related things in your patch, but nothing is actually making use of the target descriptions -- neither the core support, nor the native debugger support code is implementing the "return target's target description" hooks. Furthermore, you're using the target descriptions and calling the same initialization functions that ppc-linux-tdep.c calls. Please try an --enable-targets=all build -- I'm guessing you'll see multiple-definition link errors. -- Pedro Alves