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

[Andrew Cagney <cagney@gnu.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