Re: [patch] ia64: Fix breakpoints memory shadow

[Joel Brobecker <brobecker@adacore.com>]

> Still I rather found another defect compared to mem-break.c in the ia64
> breakpoints code.  It is another problem unrelated to the original intention
> of this ia64 patch.

Jan, please don't take it personally, I really appreciate the way
you are trying to make your patch better and better.  But since
this was unrelated, I really wished that you committed the first part
as approved, and then sent a followup patch that dealt with that part
on its own.  With the approach you took, I had to fish out the previous
version of the patch, and then compare the two patches to see exactly
what changed. And since I've always found that a diff of diff is some
kind of mental gymnastics, and the whole process is more work than just
looking at the subsequent patch, I had to delay the review until I had
30min of quiet time.

Anyway, back to the one important change:

> -  memcpy (&instr, bp_tgt->shadow_contents, sizeof instr);
> -  replace_slotN_contents (bundle, instr, slotnum);
> +  instr_breakpoint = slotN_contents (bundle_mem, slotnum);
> +  if (instr_breakpoint != IA64_BREAKPOINT)
> +    return -1;

I think we need to give the user a way to understand the error
message that returning -1 will cause. Otherwise, it's going to be
hard for the user to have a clue of why the debugger failed to
de-insert the breakpoint.  I propose a warning saying something
like this:

   cannot remove breakpoint at address %s, no break instruction at such address.

Cheers,
-- 
Joel