Re: [COMMIT] Re: [PATCH] PR gdb/15871: Unavailable entry value is not shown correctly

[Yao Qi <yao at codesourcery dot com>]

On 08/22/2013 06:04 PM, Pedro Alves wrote:
> I meant deprecated_xfer_memory, which does:
>
>         0 means that we can't handle this.  If errno has been set, it is the
>         error which prevented us from doing it (FIXME: What about bfd_error?).
>
> and using errno values as error indication in target.h methods.  E.g.,
> target_read_memory:
>
> /* Read LEN bytes of target memory at address MEMADDR, placing the results in
>     GDB's memory at MYADDR.  Returns either 0 for success or an errno value
>     if any error occurs.
>
> using errno values limits what we can return here.  There's no
> errno for <unavailable>, for example.  So, it'd be better if
> we returned target_xfer_error instead.  If we needed to indicate
> an errno or a bfd error in addition, new TARGET_XFER_E_ERRNO or
> TARGET_XFER_E_BFD values could be added to target_xfer_error
> (and the caller would then have to consult errno or bfd_error
> in addition then).

OK, it is clear to me.

I'd like to write this down in section "Internals" in 
http://sourceware.org/gdb/wiki/ProjectIdeas , let me know what do you think.

Use target_xfer_error instead of errno.  Some xfer_memory functions 
(such as target_read_memory and deprecated_xfer_memory) are
using errno to indicate any error occurs.  This limites what we can 
return here, because we can't return some GDB-specific errors, such as 
<unavailable>.  If we need to indicate an errno, bfd error for example, 
a new TARGET_XFER_E_BFD can be added to target_xfer_error.  See
http://sourceware.org/ml/gdb-patches/2013-08/msg00589.html for more info.

> > >
> > > >>+/* Report a target xfer memory error by throwing a suitable
> > > >>+   exception.  */
> > > >>+
> > > >>+static void
> > > >>+target_xfer_memory_error (enum target_xfer_error err, CORE_ADDR memaddr)
> > > >>+{
> > > >>+  switch (err)
> > > >>+    {
> > > >>+    case TARGET_XFER_E_IO:
> > > >>+      /* Actually, address between memaddr and memaddr + len was out of
> > > >>+	 bounds.  */
> > >
> > >This line of comment doesn't make much sense in the context of this
> > >function.
> I think it makes some sense if you consider the context that the caller
> always has a length handy.  But yeah.  This is a preexisting issue
> though (see memory_error, this new function can be looked at as
> a refactor).  Maybe what we should do is actually pass in the
> length, and show the range to the user instead of just the first
> address.  Sometimes that info is useful, as it may not be the first
> address in the requested range that actually is inaccessible.

Yeah, printing the range is useful some times.

--
Yao (éå)