Re: [RFA] initialize err variable in load_section_callback()

[Andrew Cagney <cagney at gnu dot org>]

Theodore A. Roth wrote:
> On Wed, 20 Oct 2004, Andrew Cagney wrote:
>
>
> > Theodore A. Roth wrote:
> >
> > > Hi,
> > >
> > > I just encountered a problem with using the "load" command with a remote
> > > avr target. The first packet would be sent to the remote target and then
> > > gdb would just give up with this error message:
> > >
> > >  (gdb) load
> > >  Loading section .text, size 0x1f8 lma 0x0
> > >  Sending packet: $M0,a:0c9446000c9463000c94#d7...Ack
> > >  Packet received: OK
> > >  Memory access error while loading section .text.
> > >
> > > It looks like load_section_callback() in symfile.c is assuming that a
> > > call to target_write_memory_partial() will set the err variable.
> > > Unfortunately, that is not a valid assumption.
> > >
> > > The attached patch got things working again, but this feels like a hack
> > > to me since target_write_memory_partial() should really be setting err
> > > to a sane value before returning.
> > >
> > > Patch is against today's cvs mainline.
> >
> > Here's the contract:
> > /* Make a single attempt at transfering LEN bytes.  On a successful
> >    transfer, the number of bytes actually transfered is returned and
> >    ERR is set to 0.  When a transfer fails, -1 is returned (the number
> >    of bytes actually transfered is not defined) and ERR is set to a
> >    non-zero error indication.  */
> > So the bug is further down the target stack.
>
>
> Both target_write_memory_partial() and target_read_memory_partial()
> break that contract then:
>
>   int
>   target_write_memory_partial (CORE_ADDR memaddr, char *buf, int len, int *err)
>   {
>     if (target_xfer_partial_p ())
>       return target_xfer_partial (target_stack, TARGET_OBJECT_MEMORY, NULL,
>                                   NULL, buf, memaddr, len);
>     else
>       return target_xfer_memory_partial (memaddr, buf, len, 1, err);
>   }
>
> If target_xfer_partial_p() returns true (which the avr port does), then
> err is never set and the caller will see garbage if it didn't initialize
> err.
>
> Should the return value of the target_xfer_partial() call be checked, or
> should err just be blindly see to zero?

The result will need to be checked, and *err set accordingly.

Hmm, to_xfer_partial doesn't specify how to handle errors.  We'd better 
pin that down.

Of hand the interface could allow:

- when -1, set *err to errno
- when -1, set *err to EIO
- when -ve, set *err -VE return value

I suspect that it should be the first.  The comments for 
target_read_partial should also be updated to mention this.

Andrew