From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7938 invoked by alias); 25 Jun 2008 13:40:54 -0000 Received: (qmail 7927 invoked by uid 22791); 25 Jun 2008 13:40:53 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 25 Jun 2008 13:40:36 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id BEA5C2A9666; Wed, 25 Jun 2008 09:40:33 -0400 (EDT) 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 MpoQa3KUqR4U; Wed, 25 Jun 2008 09:40:33 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 9EC9A2A965F; Wed, 25 Jun 2008 09:40:33 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 7018CE7ACD; Wed, 25 Jun 2008 09:40:33 -0400 (EDT) Date: Wed, 25 Jun 2008 13:51:00 -0000 From: Joel Brobecker To: Luis Machado Cc: Pedro Alves , Daniel Jacobowitz , gdb-patches@sourceware.org Subject: Re: [PATCH] PPC - Stepping off breakpoints in non-stop mode Message-ID: <20080625134033.GE3700@adacore.com> References: <1209753019.7131.29.camel@gargoyle> <20080605200249.GH25085@caradoc.them.org> <1212768014.10042.60.camel@gargoyle> <200806241503.59231.pedro@codesourcery.com> <1214317934.10496.18.camel@gargoyle> <20080625123144.GA3700@adacore.com> <1214400118.10496.36.camel@gargoyle> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1214400118.10496.36.camel@gargoyle> User-Agent: Mutt/1.4.2.2i 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: 2008-06/txt/msg00431.txt.bz2 > 2008-06-25 Luis Machado > > * rs6000-tdep.c (ppc_displaced_step_fixup): New function. > (deal_with_atomic_sequence): Update BC masks. > (rs6000_gdbarch_init): Init displaced stepping infra-structure. > Define BRANCH_MASK, B_INSN, BC_INSN, BXL_INSN, BP_MASK and BP_INSN. Yep, this looks great. Could you just double-check the formatting of a couple of comments before you commit? These darn tabs sometimes confuse me when they are combined with diff markers... > + /* Offset for non-PC relative instructions. */ Should the hyphen be between "PC" and "relative"? (I-am-not-an-English-native-speaker alert!). > + /* AA bit indicating whether this is an absolute addressing or > + PC-relative. */ I think there are a couple of spaces missing at the beginning of the second line. Also, perhaps a comment that says that the bit isn't set and therefore the addressing is [...] might be helpful? (just an idea, you don't have to followup if you disagree). > + if (debug_displaced) > + fprintf_unfiltered (gdb_stdlog, > + "displaced: (ppc) branch instruction: 0x%s\n" > + "displaced: (ppc) adjusted PC from 0x%s to 0x%s\n", > + paddr_nz (insn), paddr_nz (current_pc), > + paddr_nz (from + offset)); I missed that part in my previous review. The formatting should be: fprintf_unfiltered (gdb_stdlog, "displaced: (ppc) branch instruction: 0x%s\n" "displaced: (ppc) adjusted PC from 0x%s to 0x%s\n", paddr_nz (insn), paddr_nz (current_pc), paddr_nz (from + offset)); > + /* LK bit Indicates whether we should set the link register to point > + to the next instruction or not. */ Formatting as well on the start of the second line. -- Joel