[RFA] initialize err variable in load_section_callback()

[RFA] initialize err variable in load_section_callback()

[Theodore A. Roth]

[-- 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;
 

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

[Andrew Cagney]

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;
>  

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

[Theodore A. Roth]

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

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

[Andrew Cagney]

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

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

[Theodore A. Roth]

[-- 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);
 }

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

[Theodore A. Roth]

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

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

[Daniel Jacobowitz]

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);
 }

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

[Paul Schlie]

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.)

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

[Paul Schlie]

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.)

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

[Paul Schlie]

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.)
>