* Re: [RFA] initialize err variable in load_section_callback() @ 2004-12-02 14:45 Paul Schlie 0 siblings, 0 replies; 10+ messages in thread From: Paul Schlie @ 2004-12-02 14:45 UTC (permalink / raw) To: gdb-patches Might it be possible to "OK" the check in of Theodore Roth's load_section_callback() bug-fix patch into both 6.3 and head, that he proposed earlier in October? (as otherwise GDB can't write/update the memory contents of affected platforms/targets). http://sources.redhat.com/ml/gdb-patches/2004-10/msg00324.html (This is likely the most reliable fix for this problem, as without properly initializing the err variable introduced in this function, it's initial value is indeterminate, and otherwise fragilely relies on subsequent function calls signaling both success and failure, although traditionally only errors are typically signaled though this means.) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA] initialize err variable in load_section_callback() @ 2005-01-04 7:31 Paul Schlie 2005-01-14 23:29 ` Paul Schlie 0 siblings, 1 reply; 10+ messages in thread From: Paul Schlie @ 2005-01-04 7:31 UTC (permalink / raw) To: gdb-patches; +Cc: Theodore A. Roth, Andrew Cagney Might Ted please be given authorization to check his October patch into both the 6.3 and head branches, as it fixes an uninitialized variable problem which has already been subsequently independently found with identical fixes proposed at least a few times since: http://sources.redhat.com/ml/gdb-patches/2004-10/msg00324.html (Although there appeared to be some discussion with Andrew on the subject, this fix should be considered the least fragile way to guarantee that the err variable declared within this function's scope is initialized, as it's likely too fragile to assume that all functions which may signal errors, explicitly also signal success [other than via the absents of an error, which typically necessitates the utilized shared signaling variable be initialized as being error-free]. Where then if there is a desire to check/refined all error signaling functions within GDB such that they both explicitly signal success and failure, and guarantee that at least one such function is always called to initialize otherwise un-initialized error variables prior to being tested, this may be done independently of this proposed simple less fragile quick fix.) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA] initialize err variable in load_section_callback() 2005-01-04 7:31 Paul Schlie @ 2005-01-14 23:29 ` Paul Schlie 0 siblings, 0 replies; 10+ messages in thread From: Paul Schlie @ 2005-01-14 23:29 UTC (permalink / raw) To: gdb-patches; +Cc: Theodore A. Roth, Andrew Cagney If preferred, here's an alternate fix: In File target.c, int target_write_memory_partial (CORE_ADDR memaddr, char *buf, int len, int *err) { if (target_xfer_partial_p ()) + { + *err = 0; 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); } target_xfer_partial nor it's predecessor sets *err, therefore uninitialized. (bug introduced in version 1.83, by checkin on 9/30/04 by Andrew Cagney) > From: Paul Schlie <schlie@comcast.net> > Date: Tue, 04 Jan 2005 02:31:39 -0500 > To: <gdb-patches@sources.redhat.com> > Cc: "Theodore A. Roth" <troth@openavr.org>, Andrew Cagney <cagney@gnu.org> > Subject: Re: [RFA] initialize err variable in load_section_callback() > > Might Ted please be given authorization to check his October patch into > both the 6.3 and head branches, as it fixes an uninitialized variable problem > which has already been subsequently independently found with identical fixes > proposed at least a few times since: > > http://sources.redhat.com/ml/gdb-patches/2004-10/msg00324.html > > (Although there appeared to be some discussion with Andrew on the subject, > this fix should be considered the least fragile way to guarantee that > the err variable declared within this function's scope is initialized, > as it's likely too fragile to assume that all functions which may signal > errors, explicitly also signal success [other than via the absents of an > error, which typically necessitates the utilized shared signaling variable > be initialized as being error-free]. Where then if there is a desire to > check/refined all error signaling functions within GDB such that they both > explicitly signal success and failure, and guarantee that at least one > such function is always called to initialize otherwise un-initialized error > variables prior to being tested, this may be done independently of this > proposed simple less fragile quick fix.) > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFA] initialize err variable in load_section_callback() @ 2004-10-19 20:22 Theodore A. Roth 2004-10-20 17:34 ` Andrew Cagney 0 siblings, 1 reply; 10+ messages in thread From: Theodore A. Roth @ 2004-10-19 20:22 UTC (permalink / raw) To: gdb-patches [-- Attachment #1: Type: TEXT/PLAIN, Size: 849 bytes --] 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. --- Ted Roth PGP Key ID: 0x18F846E9 Jabber ID: troth@jabber.org [-- Attachment #2: Type: TEXT/PLAIN, Size: 787 bytes --] 2004-10-19 Theodore A. Roth <troth@openavr.org> * symfile.c (load_section_callback): Initialize err to zero since target_write_memory_partial() may not set it in all situations. Index: symfile.c =================================================================== RCS file: /cvs/src/src/gdb/symfile.c,v retrieving revision 1.143 diff -u -p -p -r1.143 symfile.c --- symfile.c 1 Oct 2004 10:23:09 -0000 1.143 +++ symfile.c 19 Oct 2004 20:07:58 -0000 @@ -1405,7 +1405,7 @@ load_section_callback (bfd *abfd, asecti struct cleanup *old_chain; CORE_ADDR lma = bfd_section_lma (abfd, asec) + args->load_offset; bfd_size_type block_size; - int err; + int err = 0; const char *sect_name = bfd_get_section_name (abfd, asec); bfd_size_type sent; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA] initialize err variable in load_section_callback() 2004-10-19 20:22 Theodore A. Roth @ 2004-10-20 17:34 ` Andrew Cagney 2004-10-20 18:07 ` Theodore A. Roth 0 siblings, 1 reply; 10+ messages in thread From: Andrew Cagney @ 2004-10-20 17:34 UTC (permalink / raw) To: Theodore A. Roth; +Cc: gdb-patches 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. Andrew > 2004-10-19 Theodore A. Roth <troth@openavr.org> > > * symfile.c (load_section_callback): Initialize err to zero since > target_write_memory_partial() may not set it in all situations. > > Index: symfile.c > =================================================================== > RCS file: /cvs/src/src/gdb/symfile.c,v > retrieving revision 1.143 > diff -u -p -p -r1.143 symfile.c > --- symfile.c 1 Oct 2004 10:23:09 -0000 1.143 > +++ symfile.c 19 Oct 2004 20:07:58 -0000 > @@ -1405,7 +1405,7 @@ load_section_callback (bfd *abfd, asecti > struct cleanup *old_chain; > CORE_ADDR lma = bfd_section_lma (abfd, asec) + args->load_offset; > bfd_size_type block_size; > - int err; > + int err = 0; > const char *sect_name = bfd_get_section_name (abfd, asec); > bfd_size_type sent; > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA] initialize err variable in load_section_callback() 2004-10-20 17:34 ` Andrew Cagney @ 2004-10-20 18:07 ` Theodore A. Roth 2004-10-26 0:04 ` Andrew Cagney 0 siblings, 1 reply; 10+ messages in thread From: Theodore A. Roth @ 2004-10-20 18:07 UTC (permalink / raw) To: Andrew Cagney; +Cc: gdb-patches 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? --- Ted Roth PGP Key ID: 0x18F846E9 Jabber ID: troth@jabber.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA] initialize err variable in load_section_callback() 2004-10-20 18:07 ` Theodore A. Roth @ 2004-10-26 0:04 ` Andrew Cagney 2004-10-26 18:19 ` Theodore A. Roth 0 siblings, 1 reply; 10+ messages in thread From: Andrew Cagney @ 2004-10-26 0:04 UTC (permalink / raw) To: Theodore A. Roth; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA] initialize err variable in load_section_callback() 2004-10-26 0:04 ` Andrew Cagney @ 2004-10-26 18:19 ` Theodore A. Roth 2004-12-28 9:10 ` Theodore A. Roth 0 siblings, 1 reply; 10+ messages in thread From: Theodore A. Roth @ 2004-10-26 18:19 UTC (permalink / raw) To: Andrew Cagney; +Cc: gdb-patches [-- Attachment #1: Type: TEXT/PLAIN, Size: 3875 bytes --] On Mon, 25 Oct 2004, Andrew Cagney wrote: > 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 Attached patch implements the above case. > - when -1, set *err to EIO I dug down the stack to see if there was a guarantee if errno is going to be set if retval -1. I didn't see that so I'm a bit nervous about my attached patch. Would it make any sense to set errno to 0 before the call to target_xfer_partial(), then if retval is -1 also check errno? I.e. if errno == 0, set *err to EIO, else *err to errno. > - when -ve, set *err -VE return value I assume -ve is an error code? Sould I extend my patch to also check for retval < -1 and if so set *err to retval? > > I suspect that it should be the first. The comments for > target_read_partial should also be updated to mention this. You lost me on this one. target_read_partial() with comments currently reads like this: /* Target vector read/write partial wrapper functions. NOTE: cagney/2003-10-21: I wonder if having "to_xfer_partial (inbuf, outbuf)", instead of separate read/write methods, make life easier. */ LONGEST target_read_partial (struct target_ops *ops, enum target_object object, const char *annex, void *buf, ULONGEST offset, LONGEST len) { return target_xfer_partial (ops, object, annex, buf, NULL, offset, len); } Was there some other comment you had in mind? Thanks for helping me with this. --- Ted Roth PGP Key ID: 0x18F846E9 Jabber ID: troth@jabber.org [-- Attachment #2: Type: TEXT/PLAIN, Size: 1786 bytes --] 2004-10-26 Theodore A. Roth <troth@openavr.org> * target.c (target_read_memory_partial): Make sure the err is set. (target_write_memory_partial): Ditto. Index: target.c =================================================================== RCS file: /cvs/src/src/gdb/target.c,v retrieving revision 1.90 diff -u -p -p -r1.90 target.c --- target.c 8 Oct 2004 20:29:55 -0000 1.90 +++ target.c 26 Oct 2004 17:35:14 -0000 @@ -1233,8 +1233,20 @@ int target_read_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, - buf, NULL, memaddr, len); + { + int retval = target_xfer_partial (target_stack, TARGET_OBJECT_MEMORY, + NULL, buf, NULL, memaddr, len); + + /* troth/2004-10-26: Are we certain that errno will be set properly if + the above call returns -1? */ + + if (retval == -1) + *err = errno; + else + *err = 0; + + return retval; + } else return target_xfer_memory_partial (memaddr, buf, len, 0, err); } @@ -1243,8 +1255,17 @@ 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); + { + int retval = target_xfer_partial (target_stack, TARGET_OBJECT_MEMORY, + NULL, NULL, buf, memaddr, len); + + if (retval == -1) + *err = errno; + else + *err = 0; + + return retval; + } else return target_xfer_memory_partial (memaddr, buf, len, 1, err); } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA] initialize err variable in load_section_callback() 2004-10-26 18:19 ` Theodore A. Roth @ 2004-12-28 9:10 ` Theodore A. Roth 2005-03-04 17:51 ` Daniel Jacobowitz 0 siblings, 1 reply; 10+ messages in thread From: Theodore A. Roth @ 2004-12-28 9:10 UTC (permalink / raw) Cc: Andrew Cagney, gdb-patches This patch got left hanging. Is it a lost cause or can I get approval to commit it? Thanks. On Tue, 26 Oct 2004, Theodore A. Roth wrote: > On Mon, 25 Oct 2004, Andrew Cagney wrote: > > > 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 > > Attached patch implements the above case. > > > - when -1, set *err to EIO > > I dug down the stack to see if there was a guarantee if errno is going > to be set if retval -1. I didn't see that so I'm a bit nervous about my > attached patch. Would it make any sense to set errno to 0 before the > call to target_xfer_partial(), then if retval is -1 also check errno? > I.e. if errno == 0, set *err to EIO, else *err to errno. > > > - when -ve, set *err -VE return value > > I assume -ve is an error code? Sould I extend my patch to also check for > retval < -1 and if so set *err to retval? > > > > > I suspect that it should be the first. The comments for > > target_read_partial should also be updated to mention this. > > You lost me on this one. target_read_partial() with comments currently > reads like this: > > /* Target vector read/write partial wrapper functions. > > NOTE: cagney/2003-10-21: I wonder if having "to_xfer_partial > (inbuf, outbuf)", instead of separate read/write methods, make life > easier. */ > > LONGEST > target_read_partial (struct target_ops *ops, > enum target_object object, > const char *annex, void *buf, > ULONGEST offset, LONGEST len) > { > return target_xfer_partial (ops, object, annex, buf, NULL, offset, len); > } > > Was there some other comment you had in mind? > > Thanks for helping me with this. > > --- > Ted Roth > PGP Key ID: 0x18F846E9 > Jabber ID: troth@jabber.org --- Ted Roth PGP Key ID: 0x18F846E9 Jabber ID: troth@jabber.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA] initialize err variable in load_section_callback() 2004-12-28 9:10 ` Theodore A. Roth @ 2005-03-04 17:51 ` Daniel Jacobowitz 0 siblings, 0 replies; 10+ messages in thread From: Daniel Jacobowitz @ 2005-03-04 17:51 UTC (permalink / raw) To: Theodore A. Roth; +Cc: Andrew Cagney, gdb-patches On Mon, Dec 27, 2004 at 06:33:48PM -0800, Theodore A. Roth wrote: > This patch got left hanging. Is it a lost cause or can I get approval to > commit it? Sorry about the delay; a coworker just ran into this, which prodded me to take another look at the problem. I'm committing something similar. Your patch isn't quite right... here's the documented behavior of target_read_memory_partial: /* 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. */ Here's a bit of target_read_partial: /* Request the transfer of up to LEN 8-bit bytes of the target's OBJECT. The OFFSET, for a seekable object, specifies the starting point. The ANNEX can be used to provide additional data-specific information to the target. Return the number of bytes actually transfered, zero when no further transfer is possible, and -1 when the transfer is not supported. "No further transfer is possible" is an error condition. remote_xfer_partial and inf_ptrace_xfer_partial don't seem to agree about whether an I/O failure is a "return 0" or a "return -1" condition; I am inclined to agree with the inf_ptrace implementation, since the transfer is supported, but failed. An argument could be made for either. This patch makes target_write_memory_partial obey its specification, as long as target_read_partial obeys its. No further transfer and unsupported are both error conditions. The specifications could still use some clarifying (separately). Tested on i686-linux native, and tested by hand that load doesn't blow up any more. > > > - when -ve, set *err -VE return value > > > > I assume -ve is an error code? Sould I extend my patch to also check for > > retval < -1 and if so set *err to retval? He just meant "negative": if the return value is negative, set *err to its absolute value. I stuck with the existing use of errno instead. -- Daniel Jacobowitz CodeSourcery, LLC 2005-03-04 Daniel Jacobowitz <dan@codesourcery.com> * target.c (target_read_memory_partial): Always initialize ERR. (target_write_memory_partial): Likewise. Index: target.c =================================================================== RCS file: /cvs/src/src/gdb/target.c,v retrieving revision 1.101 diff -u -p -r1.101 target.c --- target.c 24 Feb 2005 13:51:35 -0000 1.101 +++ target.c 4 Mar 2005 17:08:12 -0000 @@ -1,7 +1,7 @@ /* Select target systems and architectures at runtime for GDB. Copyright 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, - 1999, 2000, 2001, 2002, 2003, 2004 Free Software Foundation, Inc. + 1999, 2000, 2001, 2002, 2003, 2004, 2005 Free Software Foundation, Inc. Contributed by Cygnus Support. @@ -1249,8 +1249,26 @@ int target_read_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, - buf, NULL, memaddr, len); + { + int retval; + + retval = target_xfer_partial (target_stack, TARGET_OBJECT_MEMORY, + NULL, buf, NULL, memaddr, len); + + if (retval <= 0) + { + if (errno) + *err = errno; + else + *err = EIO; + return -1; + } + else + { + *err = 0; + return retval; + } + } else return target_xfer_memory_partial (memaddr, buf, len, 0, err); } @@ -1259,8 +1277,26 @@ 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); + { + int retval; + + retval = target_xfer_partial (target_stack, TARGET_OBJECT_MEMORY, + NULL, NULL, buf, memaddr, len); + + if (retval <= 0) + { + if (errno) + *err = errno; + else + *err = EIO; + return -1; + } + else + { + *err = 0; + return retval; + } + } else return target_xfer_memory_partial (memaddr, buf, len, 1, err); } ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2005-03-04 17:51 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2004-12-02 14:45 [RFA] initialize err variable in load_section_callback() Paul Schlie -- strict thread matches above, loose matches on Subject: below -- 2005-01-04 7:31 Paul Schlie 2005-01-14 23:29 ` Paul Schlie 2004-10-19 20:22 Theodore A. Roth 2004-10-20 17:34 ` Andrew Cagney 2004-10-20 18:07 ` Theodore A. Roth 2004-10-26 0:04 ` Andrew Cagney 2004-10-26 18:19 ` Theodore A. Roth 2004-12-28 9:10 ` Theodore A. Roth 2005-03-04 17:51 ` Daniel Jacobowitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox