From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20676 invoked by alias); 16 Mar 2011 13:58:51 -0000 Received: (qmail 20666 invoked by uid 22791); 16 Mar 2011 13:58:50 -0000 X-SWARE-Spam-Status: No, hits=-0.2 required=5.0 tests=AWL,BAYES_00,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mail2.asahi-net.or.jp (HELO mail2.asahi-net.or.jp) (202.224.39.198) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 16 Mar 2011 13:58:43 +0000 Received: from sk22g2 (y081184.ppp.asahi-net.or.jp [118.243.81.184]) by mail2.asahi-net.or.jp (Postfix) with ESMTP id 61775DA051; Wed, 16 Mar 2011 22:58:39 +0900 (JST) Received: from sa76g2.ysato.dip.jp (localhost [127.0.0.1]) by sk22g2 (Postfix) with ESMTP id 953672FEA; Wed, 16 Mar 2011 22:58:39 +0900 (JST) Date: Wed, 16 Mar 2011 14:15:00 -0000 Message-ID: <87oc5bxj3k.wl%ysato@users.sourceforge.jp> From: Yoshinori Sato To: Joel Brobecker Cc: Mike Frysinger , gdb-patches@sourceware.org Subject: Re: [PATCH] h8300 target breakpoint doesn't work on Simulator In-Reply-To: <20110311063904.GZ30306@adacore.com> References: <877hcen8ml.wl%ysato@users.sourceforge.jp> <201103041610.12924.vapier@gentoo.org> <87ipvquw41.wl%ysato@users.sourceforge.jp> <20110311063904.GZ30306@adacore.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?ISO-2022-JP-2?B?R29qGyQoRCtXGyhC?=) APEL/10.8 Emacs/23.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII 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: 2011-03/txt/msg00843.txt.bz2 At Fri, 11 Mar 2011 10:39:04 +0400, Joel Brobecker wrote: > > > +2011-03-11 Yoshinori Sato > > + > > + * h8300-tdep.c (h8300_breakpoint_from_pc): Update to breakpoint > > + instruction > > + > > A few minor comments: Thanks. > > - /*static unsigned char breakpoint[] = { 0x7A, 0xFF }; *//* ??? */ > > - static unsigned char breakpoint[] = { 0x01, 0x80 }; /* Sleep */ > > + static const unsigned char sim_breakpoint[] = { 0x7A, 0xFF }; /* bpt */ > > + static const unsigned char breakpoint[] = { 0x57, 0x30 }; /* trap #3 */ > > Can you remove a few spaces before the "/* bpt */" comment? It seems > to me that the spaces are not really necessary, and removing them > would make it a little closer to our soft-limit (70 characters). > > > - *lenptr = sizeof (breakpoint); > > - return breakpoint; > > + if (strcmp(target_shortname, "sim") == 0) { > > + *lenptr = sizeof (sim_breakpoint); > > + return sim_breakpoint; > > + } else { > > + *lenptr = sizeof (breakpoint); > > + return breakpoint; > > + } > > } > > The proper style for braces in GDB is to put the curly brace on > the next line. Also, we require a space before opening parens > in function calls. Thus > > if (strcmp (target_shortname, "sim") == 0) > { > *lenptr = sizeof (sim_breakpoint); > return sim_breakpoint; > } > else > { > *lenptr = sizeof (breakpoint); > return breakpoint; > } Sorry. I mistake editor setting. Cleanup. Index: ChangeLog =================================================================== RCS file: /cvs/src/src/gdb/ChangeLog,v retrieving revision 1.12820 diff -u -r1.12820 ChangeLog --- ChangeLog 16 Mar 2011 09:49:41 -0000 1.12820 +++ ChangeLog 16 Mar 2011 13:47:25 -0000 @@ -1,3 +1,8 @@ +2011-03-16 Yoshinori Sato + + * h8300-tdep.c (h8300_breakpoint_from_pc): Update to breakpoint + instruction + 2011-03-16 Phil Muldoon * NEWS: Add Parameter sub-classing description. Index: h8300-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/h8300-tdep.c,v retrieving revision 1.128 diff -u -r1.128 h8300-tdep.c --- h8300-tdep.c 25 Jan 2011 12:13:20 -0000 1.128 +++ h8300-tdep.c 16 Mar 2011 13:47:25 -0000 @@ -1197,11 +1197,19 @@ h8300_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, int *lenptr) { - /*static unsigned char breakpoint[] = { 0x7A, 0xFF }; *//* ??? */ - static unsigned char breakpoint[] = { 0x01, 0x80 }; /* Sleep */ + static const unsigned char sim_breakpoint[] = { 0x7A, 0xFF }; /*bpt*/ + static const unsigned char breakpoint[] = { 0x57, 0x30 }; /*trap #3*/ - *lenptr = sizeof (breakpoint); - return breakpoint; + if (strcmp(target_shortname, "sim") == 0) + { + *lenptr = sizeof (sim_breakpoint); + return sim_breakpoint; + } + else + { + *lenptr = sizeof (breakpoint); + return breakpoint; + } } static void > I wonder if there isn't a better way to detect the sim, other > than checking the target name. I don't know of any, but perhaps > other maintainers might... Hmm... I think so. However, I think it is rare case. I do not think that there is so an enhanced necessity. > -- > Joel -- Yoshinori Sato ===File ~/h8300-tdep.diff=================================== ============================================================