This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] ia64: Fix breakpoints memory shadow
> 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