[3/4] Remove a use of target_read_string

Message ID 20200612215356.22145-4-tromey@adacore.com
State New
Headers show
Series
  • Unify string-reading APIs
Related show

Commit Message

Tom Tromey June 12, 2020, 9:53 p.m.
linux-tdep.c:dump_mapping_p uses target_read_string, but in a way that
does not really make sense.  It's better to use target_read_memory
here.

gdb/ChangeLog
2020-06-02  Tom Tromey  <tromey@adacore.com>

	* linux-tdep.c (dump_mapping_p): Use target_read_memory.
---
 gdb/ChangeLog    |  4 ++++
 gdb/linux-tdep.c | 12 +++---------
 gdb/target.c     |  1 +
 3 files changed, 8 insertions(+), 9 deletions(-)

-- 
2.21.3

Comments

Simon Marchi via Gdb-patches June 13, 2020, 3:04 a.m. | #1
On Friday, June 12 2020, Tom Tromey wrote:

> linux-tdep.c:dump_mapping_p uses target_read_string, but in a way that

> does not really make sense.  It's better to use target_read_memory

> here.


Thanks for catching this problem, Tom.

> gdb/ChangeLog

> 2020-06-02  Tom Tromey  <tromey@adacore.com>

>

> 	* linux-tdep.c (dump_mapping_p): Use target_read_memory.

> ---

>  gdb/ChangeLog    |  4 ++++

>  gdb/linux-tdep.c | 12 +++---------

>  gdb/target.c     |  1 +

>  3 files changed, 8 insertions(+), 9 deletions(-)

>

> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c

> index 0f9559355f1..2dcdc630769 100644

> --- a/gdb/linux-tdep.c

> +++ b/gdb/linux-tdep.c

> @@ -701,22 +701,16 @@ dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,

>    if (!dump_p && private_p && offset == 0

>        && (filterflags & COREFILTER_ELF_HEADERS) != 0)

>      {

> -      /* Let's check if we have an ELF header.  */

> -      gdb::unique_xmalloc_ptr<char> header;

> -      int errcode;

> -

>        /* Useful define specifying the size of the ELF magical

>  	 header.  */

>  #ifndef SELFMAG

>  #define SELFMAG 4

>  #endif

>  

> -      /* Read the first SELFMAG bytes and check if it is ELFMAG.  */

> -      if (target_read_string (addr, &header, SELFMAG, &errcode) == SELFMAG

> -	  && errcode == 0)

> +      /* Let's check if we have an ELF header.  */

> +      gdb_byte h[SELFMAG];

> +      if (target_read_memory (addr, h, SELFMAG) == 0)

>  	{

> -	  const char *h = header.get ();

> -

>  	  /* The EI_MAG* and ELFMAG* constants come from

>  	     <elf/common.h>.  */

>  	  if (h[EI_MAG0] == ELFMAG0 && h[EI_MAG1] == ELFMAG1


This looks OK to me.

> diff --git a/gdb/target.c b/gdb/target.c

> index 14c494688e0..897b8fdd32b 100644

> --- a/gdb/target.c

> +++ b/gdb/target.c

> @@ -50,6 +50,7 @@

>  #include "terminal.h"

>  #include <unordered_map>

>  #include "target-connection.h"

> +#include "valprint.h"

>  

>  static void generic_tls_error (void) ATTRIBUTE_NORETURN;


This hunk looks unrelated.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/
Tom Tromey June 15, 2020, 12:13 p.m. | #2
>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@sergiodj.net> writes:


>> diff --git a/gdb/target.c b/gdb/target.c

>> index 14c494688e0..897b8fdd32b 100644

>> --- a/gdb/target.c

>> +++ b/gdb/target.c

>> @@ -50,6 +50,7 @@

>> #include "terminal.h"

>> #include <unordered_map>

>> #include "target-connection.h"

>> +#include "valprint.h"

>> 

>> static void generic_tls_error (void) ATTRIBUTE_NORETURN;


Sergio> This hunk looks unrelated.

Oops, yeah -- this should be in one of the other patches.
I'll fix this.

Tom

Patch

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 0f9559355f1..2dcdc630769 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -701,22 +701,16 @@  dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
   if (!dump_p && private_p && offset == 0
       && (filterflags & COREFILTER_ELF_HEADERS) != 0)
     {
-      /* Let's check if we have an ELF header.  */
-      gdb::unique_xmalloc_ptr<char> header;
-      int errcode;
-
       /* Useful define specifying the size of the ELF magical
 	 header.  */
 #ifndef SELFMAG
 #define SELFMAG 4
 #endif
 
-      /* Read the first SELFMAG bytes and check if it is ELFMAG.  */
-      if (target_read_string (addr, &header, SELFMAG, &errcode) == SELFMAG
-	  && errcode == 0)
+      /* Let's check if we have an ELF header.  */
+      gdb_byte h[SELFMAG];
+      if (target_read_memory (addr, h, SELFMAG) == 0)
 	{
-	  const char *h = header.get ();
-
 	  /* The EI_MAG* and ELFMAG* constants come from
 	     <elf/common.h>.  */
 	  if (h[EI_MAG0] == ELFMAG0 && h[EI_MAG1] == ELFMAG1
diff --git a/gdb/target.c b/gdb/target.c
index 14c494688e0..897b8fdd32b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -50,6 +50,7 @@ 
 #include "terminal.h"
 #include <unordered_map>
 #include "target-connection.h"
+#include "valprint.h"
 
 static void generic_tls_error (void) ATTRIBUTE_NORETURN;