From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9871 invoked by alias); 26 Oct 2004 00:04:48 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 9858 invoked from network); 26 Oct 2004 00:04:47 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 26 Oct 2004 00:04:47 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id i9Q04ff9029915 for ; Mon, 25 Oct 2004 20:04:41 -0400 Received: from localhost.redhat.com (to-dhcp51.toronto.redhat.com [172.16.14.151]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i9Q04fr19138; Mon, 25 Oct 2004 20:04:41 -0400 Received: from [127.0.0.1] (localhost.localdomain [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id BD0C11294B5; Mon, 25 Oct 2004 20:03:31 -0400 (EDT) Message-ID: <417D9450.2030401@gnu.org> Date: Tue, 26 Oct 2004 00:04:00 -0000 From: Andrew Cagney User-Agent: Mozilla Thunderbird 0.8 (X11/20041020) MIME-Version: 1.0 To: "Theodore A. Roth" Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] initialize err variable in load_section_callback() References: <4176A188.1030904@gnu.org> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2004-10/txt/msg00426.txt.bz2 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