Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Thiago Jung Bauermann <bauerman@br.ibm.com>
Cc: gdb-patches@sourceware.org,
	Luis Machado <luisgpm@linux.vnet.ibm.com>,
		Matt Tyrlik <tyrlik@us.ibm.com>
Subject: Re: [PATCH 1/4] Support the new BookE ptrace interface
Date: Tue, 05 Jan 2010 04:51:00 -0000	[thread overview]
Message-ID: <20100105045130.GA24777@adacore.com> (raw)
In-Reply-To: <200912311518.14325.bauerman@br.ibm.com>

> 2009-12-31  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
> 	    Thiago Jung Bauermann  <bauerman@br.ibm.com>
> 
> 	* ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
> 	(PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
> 	ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
> 	PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
> 	PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
> 	PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
> 	PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
> 	PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
> 	PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
> 	PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
> 	PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
> 	PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
> 	PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
> 	Define, in case <ptrace.h> doesn't provide it.
> 	(have_ptrace_new_debug_booke): New global.
> 	(ppc_linux_check_watch_resources): Renamed to ...
> 	(ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
> 	(booke_debug_info): New variable.
> 	(max_slots_number): Ditto.
> 	(hw_break_tuple): New struct.
> 	(thread_points): Ditto.
> 	(ppc_threads): New variable.
> 	(PPC_DEBUG_CURRENT_VERSION): New define.
> 	(ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
> 	(booke_cmp_hw_point): New function.
> 	(booke_find_thread_points_by_tid): Ditto.
> 	(booke_insert_point): Ditto.
> 	(booke_remove_point): Ditto.
> 	(ppc_linux_insert_hw_breakpoint): Ditto.
> 	(ppc_linux_remove_hw_breakpoint): Ditto.
> 	(ppc_linux_insert_watchpoint): Handle BookE processors.
> 	(ppc_linux_remove_watchpoint): Ditto.
> 	(ppc_linux_new_thread): Ditto.
> 	(ppc_linux_stopped_data_address): Ditto.
> 	(ppc_linux_watchpoint_addr_within_range): Ditto.
> 	(ppc_linux_read_description): Query the target to know if it
> 	supports the new kernel BookE interface.
> 	(_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
> 	to_remove_hw_breakpoint fields of the target operations struct.

Still looks good to me :). Just a couple of nits, but otherwise approved. 
It would still be nice if Ulrich had a chance to take a quick look ;-).

> +struct ppc_debug_info {
> +        uint32_t version;               /* Only version 1 exists to date */

The curly brace should be at the beginning of the next line.
And the indentation (probably a copy-paste from the kernel headers?)
needs to be adjusted.

Not that it really matters a whole lot. For this section, if it makes
things simpler to preserve the indentation of the original, I could
be convinced to leave the code untouched.

> +static int get_trigger_type (int rw)

Formatting nit as well:

   static int
   get_trigger_type (int rw)

-- 
Joel


  reply	other threads:[~2010-01-05  4:51 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-24  0:31 Thiago Jung Bauermann
2009-12-29  6:12 ` Joel Brobecker
2009-12-30 19:51   ` Thiago Jung Bauermann
2010-01-04 17:33 ` Thiago Jung Bauermann
2010-01-05  4:51   ` Joel Brobecker [this message]
2010-01-15 17:41     ` Luis Machado
2010-01-16  3:09     ` Luis Machado
2010-01-18  4:16       ` Joel Brobecker
2010-02-11 17:06     ` Ulrich Weigand
2010-02-22 20:27       ` Thiago Jung Bauermann
2010-02-23 19:07         ` Ulrich Weigand
2010-02-23 23:42           ` Joel Brobecker
2010-02-25 18:16             ` Ulrich Weigand
2010-04-22 22:53           ` Thiago Jung Bauermann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100105045130.GA24777@adacore.com \
    --to=brobecker@adacore.com \
    --cc=bauerman@br.ibm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=luisgpm@linux.vnet.ibm.com \
    --cc=tyrlik@us.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox