From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20219 invoked by alias); 11 Mar 2011 06:39:28 -0000 Received: (qmail 20209 invoked by uid 22791); 11 Mar 2011 06:39:27 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 11 Mar 2011 06:39:21 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 87EEF2BB136; Fri, 11 Mar 2011 01:39:19 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id jplnE+mMj3c4; Fri, 11 Mar 2011 01:39:19 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id AC9122BB134; Fri, 11 Mar 2011 01:39:18 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id D21851459AD; Fri, 11 Mar 2011 10:39:04 +0400 (RET) Date: Fri, 11 Mar 2011 06:58:00 -0000 From: Joel Brobecker To: Yoshinori Sato Cc: Mike Frysinger , gdb-patches@sourceware.org Subject: Re: [PATCH] h8300 target breakpoint doesn't work on Simulator Message-ID: <20110311063904.GZ30306@adacore.com> References: <877hcen8ml.wl%ysato@users.sourceforge.jp> <201103041610.12924.vapier@gentoo.org> <87ipvquw41.wl%ysato@users.sourceforge.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ipvquw41.wl%ysato@users.sourceforge.jp> User-Agent: Mutt/1.5.20 (2009-06-14) 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/msg00652.txt.bz2 > +2011-03-11 Yoshinori Sato > + > + * h8300-tdep.c (h8300_breakpoint_from_pc): Update to breakpoint > + instruction > + A few minor comments: > - /*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; } 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... -- Joel