public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Sergio Durigan Junior <sergiodj@sergiodj.net>
To: Tom Tromey <tromey@adacore.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 3/4] Remove a use of target_read_string
Date: Fri, 12 Jun 2020 23:04:51 -0400	[thread overview]
Message-ID: <877dwb8y3w.fsf@paluero> (raw)
In-Reply-To: <20200612215356.22145-4-tromey@adacore.com> (Tom Tromey's message of "Fri, 12 Jun 2020 15:53:55 -0600")

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/

  reply	other threads:[~2020-06-13  3:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-12 21:53 [PATCH 0/4] Unify string-reading APIs Tom Tromey
2020-06-12 21:53 ` [PATCH 1/4] Remove read_memory_string Tom Tromey
2020-06-13 14:19   ` Simon Marchi
2020-06-15 12:37   ` Pedro Alves
2020-06-12 21:53 ` [PATCH 2/4] Rewrite target_read_string Tom Tromey
2020-06-12 21:53 ` [PATCH 3/4] Remove a use of target_read_string Tom Tromey
2020-06-13  3:04   ` Sergio Durigan Junior [this message]
2020-06-15 12:13     ` Tom Tromey
2020-06-12 21:53 ` [PATCH 4/4] Change target_read_string API Tom Tromey
2020-06-13 14:40   ` Simon Marchi
2020-06-13 14:40 ` [PATCH 0/4] Unify string-reading APIs Simon Marchi
2020-06-15 12:27   ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877dwb8y3w.fsf@paluero \
    --to=sergiodj@sergiodj.net \
    --cc=gdb-patches@sourceware.org \
    --cc=tromey@adacore.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).