This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] i386-tdep.c, check target_read_memory for error.
On Sun, 06 Mar 2011 19:42:34 +0100, Michael Snyder wrote:
> Jan Kratochvil wrote:
> >On Fri, 04 Mar 2011 22:37:52 +0100, Michael Snyder wrote:
> >>Call error if target_read_memory fails.
> >[...]
> >>- target_read_memory (pc, &op, 1);
> >>+ if (target_read_memory (pc, &op, 1))
> >>+ error (_("Couldn't read memory at pc (%s)"), + paddress
> >>(gdbarch, pc));
> >
> >There is the function `read_memory' for such purpose.
>
> I don't understand the objection. target_read_memory may fail and
> return an error code. Coverity reports that the return value is checked
> in 78 out of 97 calling instances. So what's wrong with checking it now?
I was suggesting that instead of the code
if (target_read_memory (pc, &op, 1))
error (_("Couldn't read memory at pc (%s)"), paddress (gdbarch, pc));
One can write a shorter code with the same effect:
read_memory (pc, &op, 1);
I did not do a real patch review and I also did not write any notice about any
review/approval.
Mark Kettenis correctly noticed that the introduced errors (either by error or
through read_memory) in some of the cases are wrong / cause regressions.
Just if in some cases the error is appropriate (I do not say in which specific
cases, if any) I was suggesting calling `read_memory' is more suitable than
the explicit `target_read_memory'+`error' calls.
Thanks,
Jan