From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24373 invoked by alias); 15 Dec 2010 19:17:25 -0000 Received: (qmail 24364 invoked by uid 22791); 15 Dec 2010 19:17:24 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 15 Dec 2010 19:17:19 +0000 Received: (qmail 17718 invoked from network); 15 Dec 2010 19:17:16 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 15 Dec 2010 19:17:16 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [PATCH v4] gdb: bfin: new port Date: Wed, 15 Dec 2010 19:17:00 -0000 User-Agent: KMail/1.13.5 (Linux/2.6.33-29-realtime; KDE/4.4.5; x86_64; ; ) Cc: Mike Frysinger , toolchain-devel@blackfin.uclinux.org, Jie Zhang References: <1292431646-3744-1-git-send-email-vapier@gentoo.org> In-Reply-To: <1292431646-3744-1-git-send-email-vapier@gentoo.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <201012151917.03834.pedro@codesourcery.com> X-IsSubscribed: yes 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: 2010-12/txt/msg00316.txt.bz2 On Wednesday 15 December 2010 16:47:25, Mike Frysinger wrote: > Signed-off-by: Jie Zhang > Signed-off-by: Mike Frysinger >=20 > 2010-12-15 Jie Zhang >=20 Should really be, IMO: 2010-12-15 Jie Zhang Mike Frysinger > +bfin-*-*) > + # Target: Blackfin > + gdb_target_obs=3D"bfin-tdep.o bfin-linux-tdep.o linux-tdep.o" > + ;; should probably be: bfin-*-uclinux*) # Target: Blackfin running uclinux gdb_target_obs=3D"bfin-tdep.o bfin-linux-tdep.o linux-tdep.o" ;; bfin-*-*) # Target: Blackfin gdb_target_obs=3D"bfin-tdep.o" ;; The top-level src/configure.ac seems to actually use bfin*-*-uclinux* (extra star). You should update the triplets in the NEWS entry as well. > +void > +_initialize_bfin_tdep (void) Needs a prototype: /* Provide a prototype to silence -Wmissing-prototypes. */ extern initialize_file_ftype _initialize_bfin_tdep; There's the equivalent one in bfin-linux-tdep.c, but you're missing it in bfin-tdep.c. > + warning (_("Function Prologue not recognised; pc will point to = ENTRY_POINT of the function")); way too long line. write as, e.g.,=20 warning (_("Function Prologue not recognised; " "pc will point to ENTRY_POINT of the function")); > + /* initialize R0, R1 and R2 to the first 3 words of paramters */ /* Initialize R0, R1 and R2 to the first 3 words of paramters. */ Capitalize, typo, period, double space. > +/* Target-dependent code for Analog Devices Blackfin processer, for GDB. typo: processor > + Copyright (C) 2005-2010 Free Software Foundation, Inc. I don't think we can use an year range in our sources: : "You can use a range (=E2=80=982008-2010=E2=80=99) instead of listing indiv= idual years (=E2=80=982008, 2009, 2010=E2=80=99) if and only if: 1) every year in= the range, inclusive, really is a =E2=80=9Ccopyrightable=E2=80=9D year that would be listed indiv= idually; and 2) you make an explicit statement in a =E2=80=98README=E2=80=99 file about this us= age. " I think we fail point #2. I see no other use of an year range in the sources. b/gdb/bfin-tdep.h: > +/* in opcodes/bfin-dis.c */ > +extern int print_insn_bfin (bfd_vma pc, struct disassemble_info *outf); Please remove this. It is declared in src/include/dis-asm.h. Otherwise, with the astat/cc change, this looks good to me. --=20 Pedro Alves