[RFA] arm-tdep.c: deal with failed memory read

[RFA] arm-tdep.c: deal with failed memory read

[Elena Zannoni]

If, upon initial connection to a remote ARM target, the contents of
r11 (which is the Frame Pointer) are junk, a memory read from
arm_scan_prologue can fail and abort the whole connection to the
remote target. There are several ways to fix this, and probably the
most correct one is to teach gdb to do the initial connection in 2
separate steps. First connect and declare that successful or not, then
start reading memory if the connection was established.

This patch is just a band-aid to allow intercepting bad memory reads
and not aborting the connection.  It has been in our internal
repository for a couple of months now. It is by no means a complete
solution, but it improves things a bit.

OK?

Elena


2001-11-21  Elena Zannoni  <ezannoni@redhat.com>

        * corefile.c (do_captured_read_memory_integer,
        gdb_read_memory_integer): New functions.
        * gdbcore.h (gdb_read_memory_integer): Export.
        * arm-tdep.c (arm_scan_prologue): Use gdb_read_memory_integer,
        to read the frame value, to capture calls to error().

Index: arm-tdep.c
===================================================================
RCS file: /cvs/uberbaum/gdb/arm-tdep.c,v
retrieving revision 1.17
diff -u -p -r1.17 arm-tdep.c
--- arm-tdep.c	2001/11/14 08:18:32	1.17
+++ arm-tdep.c	2001/11/22 00:08:28
@@ -717,6 +717,7 @@ static void
 arm_scan_prologue (struct frame_info *fi)
 {
   int regno, sp_offset, fp_offset;
+  LONGEST return_value;
   CORE_ADDR prologue_start, prologue_end, current_pc;
 
   /* Check if this function is already in the cache of frame information. */
@@ -781,9 +782,13 @@ arm_scan_prologue (struct frame_info *fi
     {
       /* Get address of the stmfd in the prologue of the callee; the saved
          PC is the address of the stmfd + 8.  */
-      prologue_start = ADDR_BITS_REMOVE (read_memory_integer (fi->frame, 4))
-	- 8;
-      prologue_end = prologue_start + 64;	/* See above. */
+      if (!gdb_read_memory_integer (fi->frame, 4,  &return_value))
+	return;
+      else
+	{
+	  prologue_start = ADDR_BITS_REMOVE (return_value) - 8;
+	  prologue_end = prologue_start + 64;	/* See above. */
+	}
     }
 
   /* Now search the prologue looking for instructions that set up the
Index: corefile.c
===================================================================
RCS file: /cvs/uberbaum/gdb/corefile.c,v
retrieving revision 1.15
diff -u -p -r1.15 corefile.c
--- corefile.c	2001/11/12 21:08:04	1.15
+++ corefile.c	2001/11/22 00:08:50
@@ -262,6 +262,41 @@ dis_asm_print_address (bfd_vma addr, str
 
 /* Read an integer from debugged memory, given address and number of bytes.  */
 
+struct captured_read_memory_integer_arguments
+{
+  CORE_ADDR memaddr;
+  int len;
+  LONGEST result;
+};
+
+static int
+do_captured_read_memory_integer (void *data)
+{
+  struct captured_read_memory_integer_arguments *args = (struct captured_read_memory_integer_arguments*) data
;
+  CORE_ADDR memaddr = args->memaddr;
+  int len = args->len;
+
+  args->result = read_memory_integer (memaddr, len);
+
+  return 0;
+}
+
+int
+gdb_read_memory_integer (CORE_ADDR memaddr, int len, LONGEST *return_value)
+{
+  int status;
+  struct captured_read_memory_integer_arguments args;
+  args.memaddr = memaddr;
+  args.len = len;
+
+  status = catch_errors (do_captured_read_memory_integer, &args,
+			 "", RETURN_MASK_ALL);
+  if (!status)
+    *return_value = args.result;
+
+  return status;
+}
+
 LONGEST
 read_memory_integer (CORE_ADDR memaddr, int len)
 {
Index: gdbcore.h
===================================================================
RCS file: /cvs/uberbaum/gdb/gdbcore.h,v
retrieving revision 1.8
diff -u -p -r1.8 gdbcore.h
--- gdbcore.h	2001/11/12 21:08:04	1.8
+++ gdbcore.h	2001/11/22 00:09:12
@@ -55,6 +55,7 @@ extern void read_memory (CORE_ADDR memad
    bytes.  */
 
 extern LONGEST read_memory_integer (CORE_ADDR memaddr, int len);
+extern int gdb_read_memory_integer (CORE_ADDR memaddr, int len, LONGEST *return_value);
 
 /* Read an unsigned integer from debugged memory, given address and
    number of bytes.  */

Re: [RFA] arm-tdep.c: deal with failed memory read

[Fernando Nasser]

Elena Zannoni wrote:
> 
> If, upon initial connection to a remote ARM target, the contents of
> r11 (which is the Frame Pointer) are junk, a memory read from
> arm_scan_prologue can fail and abort the whole connection to the
> remote target. There are several ways to fix this, and probably the
> most correct one is to teach gdb to do the initial connection in 2
> separate steps. First connect and declare that successful or not, then
> start reading memory if the connection was established.
> 
> This patch is just a band-aid to allow intercepting bad memory reads
> and not aborting the connection.  It has been in our internal
> repository for a couple of months now. It is by no means a complete
> solution, but it improves things a bit.
> 
> OK?
> 

The arm-tdep.c part is approved.

Fernando


> Elena
> 
> 2001-11-21  Elena Zannoni  <ezannoni@redhat.com>
> 
>         * corefile.c (do_captured_read_memory_integer,
>         gdb_read_memory_integer): New functions.
>         * gdbcore.h (gdb_read_memory_integer): Export.
>         * arm-tdep.c (arm_scan_prologue): Use gdb_read_memory_integer,
>         to read the frame value, to capture calls to error().
> 
> Index: arm-tdep.c
> ===================================================================
> RCS file: /cvs/uberbaum/gdb/arm-tdep.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 arm-tdep.c
> --- arm-tdep.c  2001/11/14 08:18:32     1.17
> +++ arm-tdep.c  2001/11/22 00:08:28
> @@ -717,6 +717,7 @@ static void
>  arm_scan_prologue (struct frame_info *fi)
>  {
>    int regno, sp_offset, fp_offset;
> +  LONGEST return_value;
>    CORE_ADDR prologue_start, prologue_end, current_pc;
> 
>    /* Check if this function is already in the cache of frame information. */
> @@ -781,9 +782,13 @@ arm_scan_prologue (struct frame_info *fi
>      {
>        /* Get address of the stmfd in the prologue of the callee; the saved
>           PC is the address of the stmfd + 8.  */
> -      prologue_start = ADDR_BITS_REMOVE (read_memory_integer (fi->frame, 4))
> -       - 8;
> -      prologue_end = prologue_start + 64;      /* See above. */
> +      if (!gdb_read_memory_integer (fi->frame, 4,  &return_value))
> +       return;
> +      else
> +       {
> +         prologue_start = ADDR_BITS_REMOVE (return_value) - 8;
> +         prologue_end = prologue_start + 64;   /* See above. */
> +       }
>      }
> 
>    /* Now search the prologue looking for instructions that set up the
> Index: corefile.c
> ===================================================================
> RCS file: /cvs/uberbaum/gdb/corefile.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 corefile.c
> --- corefile.c  2001/11/12 21:08:04     1.15
> +++ corefile.c  2001/11/22 00:08:50
> @@ -262,6 +262,41 @@ dis_asm_print_address (bfd_vma addr, str
> 
>  /* Read an integer from debugged memory, given address and number of bytes.  */
> 
> +struct captured_read_memory_integer_arguments
> +{
> +  CORE_ADDR memaddr;
> +  int len;
> +  LONGEST result;
> +};
> +
> +static int
> +do_captured_read_memory_integer (void *data)
> +{
> +  struct captured_read_memory_integer_arguments *args = (struct captured_read_memory_integer_arguments*) data
> ;
> +  CORE_ADDR memaddr = args->memaddr;
> +  int len = args->len;
> +
> +  args->result = read_memory_integer (memaddr, len);
> +
> +  return 0;
> +}
> +
> +int
> +gdb_read_memory_integer (CORE_ADDR memaddr, int len, LONGEST *return_value)
> +{
> +  int status;
> +  struct captured_read_memory_integer_arguments args;
> +  args.memaddr = memaddr;
> +  args.len = len;
> +
> +  status = catch_errors (do_captured_read_memory_integer, &args,
> +                        "", RETURN_MASK_ALL);
> +  if (!status)
> +    *return_value = args.result;
> +
> +  return status;
> +}
> +
>  LONGEST
>  read_memory_integer (CORE_ADDR memaddr, int len)
>  {
> Index: gdbcore.h
> ===================================================================
> RCS file: /cvs/uberbaum/gdb/gdbcore.h,v
> retrieving revision 1.8
> diff -u -p -r1.8 gdbcore.h
> --- gdbcore.h   2001/11/12 21:08:04     1.8
> +++ gdbcore.h   2001/11/22 00:09:12
> @@ -55,6 +55,7 @@ extern void read_memory (CORE_ADDR memad
>     bytes.  */
> 
>  extern LONGEST read_memory_integer (CORE_ADDR memaddr, int len);
> +extern int gdb_read_memory_integer (CORE_ADDR memaddr, int len, LONGEST *return_value);
> 
>  /* Read an unsigned integer from debugged memory, given address and
>     number of bytes.  */

-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] arm-tdep.c: deal with failed memory read

[Andrew Cagney]

> Elena Zannoni wrote:
> 
>> 
>> If, upon initial connection to a remote ARM target, the contents of
>> r11 (which is the Frame Pointer) are junk, a memory read from
>> arm_scan_prologue can fail and abort the whole connection to the
>> remote target. There are several ways to fix this, and probably the
>> most correct one is to teach gdb to do the initial connection in 2
>> separate steps. First connect and declare that successful or not, then
>> start reading memory if the connection was established.
>> 
>> This patch is just a band-aid to allow intercepting bad memory reads
>> and not aborting the connection.  It has been in our internal
>> repository for a couple of months now. It is by no means a complete
>> solution, but it improves things a bit.
>> 
>> OK?
>> 
> 
> 
> The arm-tdep.c part is approved.

We desperatly need a better naming convention and clearer semantics 
(what happens if the function fails due to a target disconnect) for 
these wrapped functions.  gdb_*() is being used by both libgdb and 
wrapper.[hc] et.al.

Suggestions?

Otherwize ok.

	Andrew


>> Elena
>> 
>> 2001-11-21  Elena Zannoni  <ezannoni@redhat.com>
>> 
>> * corefile.c (do_captured_read_memory_integer,
>> gdb_read_memory_integer): New functions.
>> * gdbcore.h (gdb_read_memory_integer): Export.
>> * arm-tdep.c (arm_scan_prologue): Use gdb_read_memory_integer,
>> to read the frame value, to capture calls to error().
>> 
>> Index: arm-tdep.c
>> ===================================================================
>> RCS file: /cvs/uberbaum/gdb/arm-tdep.c,v
>> retrieving revision 1.17
>> diff -u -p -r1.17 arm-tdep.c
>> --- arm-tdep.c  2001/11/14 08:18:32     1.17
>> +++ arm-tdep.c  2001/11/22 00:08:28
>> @@ -717,6 +717,7 @@ static void
>> arm_scan_prologue (struct frame_info *fi)
>> {
>> int regno, sp_offset, fp_offset;
>> +  LONGEST return_value;
>> CORE_ADDR prologue_start, prologue_end, current_pc;
>> 
>> /* Check if this function is already in the cache of frame information. */
>> @@ -781,9 +782,13 @@ arm_scan_prologue (struct frame_info *fi
>> {
>> /* Get address of the stmfd in the prologue of the callee; the saved
>> PC is the address of the stmfd + 8.  */
>> -      prologue_start = ADDR_BITS_REMOVE (read_memory_integer (fi->frame, 4))
>> -       - 8;
>> -      prologue_end = prologue_start + 64;      /* See above. */
>> +      if (!gdb_read_memory_integer (fi->frame, 4,  &return_value))
>> +       return;
>> +      else
>> +       {
>> +         prologue_start = ADDR_BITS_REMOVE (return_value) - 8;
>> +         prologue_end = prologue_start + 64;   /* See above. */
>> +       }
>> }
>> 
>> /* Now search the prologue looking for instructions that set up the
>> Index: corefile.c
>> ===================================================================
>> RCS file: /cvs/uberbaum/gdb/corefile.c,v
>> retrieving revision 1.15
>> diff -u -p -r1.15 corefile.c
>> --- corefile.c  2001/11/12 21:08:04     1.15
>> +++ corefile.c  2001/11/22 00:08:50
>> @@ -262,6 +262,41 @@ dis_asm_print_address (bfd_vma addr, str
>> 
>> /* Read an integer from debugged memory, given address and number of bytes.  */
>> 
>> +struct captured_read_memory_integer_arguments
>> +{
>> +  CORE_ADDR memaddr;
>> +  int len;
>> +  LONGEST result;
>> +};
>> +
>> +static int
>> +do_captured_read_memory_integer (void *data)
>> +{
>> +  struct captured_read_memory_integer_arguments *args = (struct captured_read_memory_integer_arguments*) data
>> ;
>> +  CORE_ADDR memaddr = args->memaddr;
>> +  int len = args->len;
>> +
>> +  args->result = read_memory_integer (memaddr, len);
>> +
>> +  return 0;
>> +}
>> +
>> +int
>> +gdb_read_memory_integer (CORE_ADDR memaddr, int len, LONGEST *return_value)
>> +{
>> +  int status;
>> +  struct captured_read_memory_integer_arguments args;
>> +  args.memaddr = memaddr;
>> +  args.len = len;
>> +
>> +  status = catch_errors (do_captured_read_memory_integer, &args,
>> +                        "", RETURN_MASK_ALL);
>> +  if (!status)
>> +    *return_value = args.result;
>> +
>> +  return status;
>> +}
>> +
>> LONGEST
>> read_memory_integer (CORE_ADDR memaddr, int len)
>> {
>> Index: gdbcore.h
> 

Re: [RFA] arm-tdep.c: deal with failed memory read

[Andrew Cagney]

> The arm-tdep.c part is approved.
> 
> We desperatly need a better naming convention and clearer semantics (what happens if the function fails due to a target disconnect) for these wrapped functions.  gdb_*() is being used by both libgdb and wrapper.[hc] et.al.

Hmm, this doesn't read very well.  Lets try ...

gdb.h contains gdb_...() libgdb functions.

wrapper.h contains gdb_...() save functions.

Two very different interfaces with identical prefixes.  I think a 
separate naming convention needs to be adopted for save / wrapped / ... 
functions.  I also think the function semantics need to be more tightly 
defined.  For instance, a safe function should catch a bad memory read, 
should that safe function catch a failure because the target interface 
has gone down (tcp connection lost, ...) or because the user entered a 
cntrl-c.

Anyway, food for thought.

enjoy,
Andrew

Re: [RFA] arm-tdep.c: deal with failed memory read

[Andrew Cagney]

> Elena Zannoni wrote:
> 
>> 
>> If, upon initial connection to a remote ARM target, the contents of
>> r11 (which is the Frame Pointer) are junk, a memory read from
>> arm_scan_prologue can fail and abort the whole connection to the
>> remote target. There are several ways to fix this, and probably the
>> most correct one is to teach gdb to do the initial connection in 2
>> separate steps. First connect and declare that successful or not, then
>> start reading memory if the connection was established.
>> 
>> This patch is just a band-aid to allow intercepting bad memory reads
>> and not aborting the connection.  It has been in our internal
>> repository for a couple of months now. It is by no means a complete
>> solution, but it improves things a bit.
>> 
>> OK?
>> 
> 
> 
> The arm-tdep.c part is approved.

We desperatly need a better naming convention and clearer semantics 
(what happens if the function fails due to a target disconnect) for 
these wrapped functions.  gdb_*() is being used by both libgdb and 
wrapper.[hc] et.al.

Suggestions?

Otherwize ok.

	Andrew


>> Elena
>> 
>> 2001-11-21  Elena Zannoni  <ezannoni@redhat.com>
>> 
>> * corefile.c (do_captured_read_memory_integer,
>> gdb_read_memory_integer): New functions.
>> * gdbcore.h (gdb_read_memory_integer): Export.
>> * arm-tdep.c (arm_scan_prologue): Use gdb_read_memory_integer,
>> to read the frame value, to capture calls to error().
>> 
>> Index: arm-tdep.c
>> ===================================================================
>> RCS file: /cvs/uberbaum/gdb/arm-tdep.c,v
>> retrieving revision 1.17
>> diff -u -p -r1.17 arm-tdep.c
>> --- arm-tdep.c  2001/11/14 08:18:32     1.17
>> +++ arm-tdep.c  2001/11/22 00:08:28
>> @@ -717,6 +717,7 @@ static void
>> arm_scan_prologue (struct frame_info *fi)
>> {
>> int regno, sp_offset, fp_offset;
>> +  LONGEST return_value;
>> CORE_ADDR prologue_start, prologue_end, current_pc;
>> 
>> /* Check if this function is already in the cache of frame information. */
>> @@ -781,9 +782,13 @@ arm_scan_prologue (struct frame_info *fi
>> {
>> /* Get address of the stmfd in the prologue of the callee; the saved
>> PC is the address of the stmfd + 8.  */
>> -      prologue_start = ADDR_BITS_REMOVE (read_memory_integer (fi->frame, 4))
>> -       - 8;
>> -      prologue_end = prologue_start + 64;      /* See above. */
>> +      if (!gdb_read_memory_integer (fi->frame, 4,  &return_value))
>> +       return;
>> +      else
>> +       {
>> +         prologue_start = ADDR_BITS_REMOVE (return_value) - 8;
>> +         prologue_end = prologue_start + 64;   /* See above. */
>> +       }
>> }
>> 
>> /* Now search the prologue looking for instructions that set up the
>> Index: corefile.c
>> ===================================================================
>> RCS file: /cvs/uberbaum/gdb/corefile.c,v
>> retrieving revision 1.15
>> diff -u -p -r1.15 corefile.c
>> --- corefile.c  2001/11/12 21:08:04     1.15
>> +++ corefile.c  2001/11/22 00:08:50
>> @@ -262,6 +262,41 @@ dis_asm_print_address (bfd_vma addr, str
>> 
>> /* Read an integer from debugged memory, given address and number of bytes.  */
>> 
>> +struct captured_read_memory_integer_arguments
>> +{
>> +  CORE_ADDR memaddr;
>> +  int len;
>> +  LONGEST result;
>> +};
>> +
>> +static int
>> +do_captured_read_memory_integer (void *data)
>> +{
>> +  struct captured_read_memory_integer_arguments *args = (struct captured_read_memory_integer_arguments*) data
>> ;
>> +  CORE_ADDR memaddr = args->memaddr;
>> +  int len = args->len;
>> +
>> +  args->result = read_memory_integer (memaddr, len);
>> +
>> +  return 0;
>> +}
>> +
>> +int
>> +gdb_read_memory_integer (CORE_ADDR memaddr, int len, LONGEST *return_value)
>> +{
>> +  int status;
>> +  struct captured_read_memory_integer_arguments args;
>> +  args.memaddr = memaddr;
>> +  args.len = len;
>> +
>> +  status = catch_errors (do_captured_read_memory_integer, &args,
>> +                        "", RETURN_MASK_ALL);
>> +  if (!status)
>> +    *return_value = args.result;
>> +
>> +  return status;
>> +}
>> +
>> LONGEST
>> read_memory_integer (CORE_ADDR memaddr, int len)
>> {
>> Index: gdbcore.h
> 

Re: [RFA] arm-tdep.c: deal with failed memory read

[Andrew Cagney]

> The arm-tdep.c part is approved.
> 
> We desperatly need a better naming convention and clearer semantics (what happens if the function fails due to a target disconnect) for these wrapped functions.  gdb_*() is being used by both libgdb and wrapper.[hc] et.al.

Hmm, this doesn't read very well.  Lets try ...

gdb.h contains gdb_...() libgdb functions.

wrapper.h contains gdb_...() save functions.

Two very different interfaces with identical prefixes.  I think a 
separate naming convention needs to be adopted for save / wrapped / ... 
functions.  I also think the function semantics need to be more tightly 
defined.  For instance, a safe function should catch a bad memory read, 
should that safe function catch a failure because the target interface 
has gone down (tcp connection lost, ...) or because the user entered a 
cntrl-c.

Anyway, food for thought.

enjoy,
Andrew

Re: [RFA] arm-tdep.c: deal with failed memory read

[Elena Zannoni]

Andrew Cagney writes:
 > > Elena Zannoni wrote:
 > > 
 > >> 
 > >> If, upon initial connection to a remote ARM target, the contents of
 > >> r11 (which is the Frame Pointer) are junk, a memory read from
 > >> arm_scan_prologue can fail and abort the whole connection to the
 > >> remote target. There are several ways to fix this, and probably the
 > >> most correct one is to teach gdb to do the initial connection in 2
 > >> separate steps. First connect and declare that successful or not, then
 > >> start reading memory if the connection was established.
 > >> 
 > >> This patch is just a band-aid to allow intercepting bad memory reads
 > >> and not aborting the connection.  It has been in our internal
 > >> repository for a couple of months now. It is by no means a complete
 > >> solution, but it improves things a bit.
 > >> 
 > >> OK?
 > >> 
 > > 
 > > 
 > > The arm-tdep.c part is approved.
 > 
 > We desperatly need a better naming convention and clearer semantics 
 > (what happens if the function fails due to a target disconnect) for 
 > these wrapped functions.  gdb_*() is being used by both libgdb and 
 > wrapper.[hc] et.al.
 > 
 > Suggestions?

I can change the name of the functions in wrapper (and this one) to
something like safe_read_memory_integer if that's a name people prefer.

I can submit separate patches to do the other functions.

Elena


 > 
 > Otherwize ok.
 > 
 > 	Andrew
 > 
 > 
 > >> Elena
 > >> 
 > >> 2001-11-21  Elena Zannoni  <ezannoni@redhat.com>
 > >> 
 > >> * corefile.c (do_captured_read_memory_integer,
 > >> gdb_read_memory_integer): New functions.
 > >> * gdbcore.h (gdb_read_memory_integer): Export.
 > >> * arm-tdep.c (arm_scan_prologue): Use gdb_read_memory_integer,
 > >> to read the frame value, to capture calls to error().
 > >> 
 > >> Index: arm-tdep.c
 > >> ===================================================================
 > >> RCS file: /cvs/uberbaum/gdb/arm-tdep.c,v
 > >> retrieving revision 1.17
 > >> diff -u -p -r1.17 arm-tdep.c
 > >> --- arm-tdep.c  2001/11/14 08:18:32     1.17
 > >> +++ arm-tdep.c  2001/11/22 00:08:28
 > >> @@ -717,6 +717,7 @@ static void
 > >> arm_scan_prologue (struct frame_info *fi)
 > >> {
 > >> int regno, sp_offset, fp_offset;
 > >> +  LONGEST return_value;
 > >> CORE_ADDR prologue_start, prologue_end, current_pc;
 > >> 
 > >> /* Check if this function is already in the cache of frame information. */
 > >> @@ -781,9 +782,13 @@ arm_scan_prologue (struct frame_info *fi
 > >> {
 > >> /* Get address of the stmfd in the prologue of the callee; the saved
 > >> PC is the address of the stmfd + 8.  */
 > >> -      prologue_start = ADDR_BITS_REMOVE (read_memory_integer (fi->frame, 4))
 > >> -       - 8;
 > >> -      prologue_end = prologue_start + 64;      /* See above. */
 > >> +      if (!gdb_read_memory_integer (fi->frame, 4,  &return_value))
 > >> +       return;
 > >> +      else
 > >> +       {
 > >> +         prologue_start = ADDR_BITS_REMOVE (return_value) - 8;
 > >> +         prologue_end = prologue_start + 64;   /* See above. */
 > >> +       }
 > >> }
 > >> 
 > >> /* Now search the prologue looking for instructions that set up the
 > >> Index: corefile.c
 > >> ===================================================================
 > >> RCS file: /cvs/uberbaum/gdb/corefile.c,v
 > >> retrieving revision 1.15
 > >> diff -u -p -r1.15 corefile.c
 > >> --- corefile.c  2001/11/12 21:08:04     1.15
 > >> +++ corefile.c  2001/11/22 00:08:50
 > >> @@ -262,6 +262,41 @@ dis_asm_print_address (bfd_vma addr, str
 > >> 
 > >> /* Read an integer from debugged memory, given address and number of bytes.  */
 > >> 
 > >> +struct captured_read_memory_integer_arguments
 > >> +{
 > >> +  CORE_ADDR memaddr;
 > >> +  int len;
 > >> +  LONGEST result;
 > >> +};
 > >> +
 > >> +static int
 > >> +do_captured_read_memory_integer (void *data)
 > >> +{
 > >> +  struct captured_read_memory_integer_arguments *args = (struct captured_read_memory_integer_arguments*) data
 > >> ;
 > >> +  CORE_ADDR memaddr = args->memaddr;
 > >> +  int len = args->len;
 > >> +
 > >> +  args->result = read_memory_integer (memaddr, len);
 > >> +
 > >> +  return 0;
 > >> +}
 > >> +
 > >> +int
 > >> +gdb_read_memory_integer (CORE_ADDR memaddr, int len, LONGEST *return_value)
 > >> +{
 > >> +  int status;
 > >> +  struct captured_read_memory_integer_arguments args;
 > >> +  args.memaddr = memaddr;
 > >> +  args.len = len;
 > >> +
 > >> +  status = catch_errors (do_captured_read_memory_integer, &args,
 > >> +                        "", RETURN_MASK_ALL);
 > >> +  if (!status)
 > >> +    *return_value = args.result;
 > >> +
 > >> +  return status;
 > >> +}
 > >> +
 > >> LONGEST
 > >> read_memory_integer (CORE_ADDR memaddr, int len)
 > >> {
 > >> Index: gdbcore.h
 > > 
 > 

Re: [RFA] arm-tdep.c: deal with failed memory read

[Elena Zannoni]

Andrew Cagney writes:
 > > The arm-tdep.c part is approved.
 > > 
 > > We desperatly need a better naming convention and clearer semantics (what happens if the function fails due to a target disconnect) for these wrapped functions.  gdb_*() is being used by both libgdb and wrapper.[hc] et.al.
 > 
 > Hmm, this doesn't read very well.  Lets try ...
 > 
 > gdb.h contains gdb_...() libgdb functions.
 > 
 > wrapper.h contains gdb_...() save functions.
 > 
 > Two very different interfaces with identical prefixes.  I think a 
 > separate naming convention needs to be adopted for save / wrapped / ... 
 > functions.  I also think the function semantics need to be more tightly 
 > defined.  For instance, a safe function should catch a bad memory read, 
 > should that safe function catch a failure because the target interface 
 > has gone down (tcp connection lost, ...) or because the user entered a 
 > cntrl-c.
 > 
 > Anyway, food for thought.
 > 
 > enjoy,
 > Andrew
 > 

I committed the patch and changed the name from gdb_read_memory_integer
to safe_read_memory_integer.

I haven't committed it to the 5.1 branch, should I?.

Elena