This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] ia64: Fix breakpoints memory shadow


> Date: Thu, 20 Nov 2008 15:49:36 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> gdb/doc/
> 2008-11-20  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdbint.texinfo (Target Conditionals): Extend the
> 	gdbarch_breakpoint_from_pc description.

This part is approved, thanks.

However, I'm slightly worried by these parts in your code:

> +  instr_breakpoint = slotN_contents (bundle_mem, slotnum);
> +  if (instr_breakpoint != IA64_BREAKPOINT)
> +    warning (_("Breakpoint removal cannot find the placed breakpoint at %s"),
> +             paddr_nz (bp_tgt->placed_address));

Can this happen as part of normal GDB behavior?  If not, we should
make this internal_error, I think.  If indeed this is a warning, we
should tell users what to do with such a warning, either as part of
the message or at least in the manual.

> +  if (slotnum > 2)
> +    error (_("Can't insert breakpoint for slot numbers greater than 2."));

Similarly here: assuming slot numbers are not something exposed to the
user, I'd be bewildered if presented with such a message.  If this is
an internal GDB error (a.k.a. bug), let's treat it like one.

(Yes, I do realize that there's already a similar call to `error'
elsewhere in the existing GDB code.  My comments apply to that place
as well.)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]