Re: [patch] ia64: Fix breakpoints memory shadow

Eli Zaretskii
2008-11-22T11:30:59Z

> 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.)