diff mbox series

[v2,3/6] gdb: introduce helper class file_reader_t

Message ID 20260825100912.514232-4-matthieu.longo@arm.com
State New
Headers
Series gdb: introduce file_reader_t to read procfs files |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 fail Patch failed to apply

Commit Message

Matthieu Longo Aug. 25, 2026, 10:09 a.m. UTC 
  Wrap all the boilerplate code required to read a file in a new helper
class: file_reader_t. The class owns the file contents together with
the file path, and provides convenient accessors for the data, size and
typed views. It supports both null-terminated text files and binary files.

This helper eliminates repeated calls to target_fileio_read_stralloc
and target_fileio_read_alloc, remove explicit memory management with
gdb::unique_xmalloc_ptr, and simplifies the casting logic when working
with binary data.

The patch converts some of the existing Linux, AMD64, and SPARC code that
reads files from /proc to use file_reader_t. As a side effect,
amd64_linux_lam_untag_mask and linux_process_address_in_memtag_page
may now return earlier in case the file is empty.

Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Reviewed-By: Christina Joos <christina.joos@intel.com>
---
 gdb/amd64-linux-tdep.c |  12 ++--
 gdb/linux-tdep.c       | 122 ++++++++++++++++++-----------------------
 gdb/sparc64-tdep.c     |  15 +++--
 gdb/target.h           |  78 ++++++++++++++++++++++++++
 4 files changed, 143 insertions(+), 84 deletions(-)

Comments

Andrew Burgess Sept. 10, 2026, 3:35 p.m. UTC | #1 
Matthieu Longo <matthieu.longo@arm.com> writes:

> Wrap all the boilerplate code required to read a file in a new helper
> class: file_reader_t. The class owns the file contents together with
> the file path, and provides convenient accessors for the data, size and
> typed views. It supports both null-terminated text files and binary files.
>
> This helper eliminates repeated calls to target_fileio_read_stralloc
> and target_fileio_read_alloc, remove explicit memory management with

s/remove/removes/

> gdb::unique_xmalloc_ptr, and simplifies the casting logic when working
> with binary data.
>
> The patch converts some of the existing Linux, AMD64, and SPARC code that
> reads files from /proc to use file_reader_t. As a side effect,
> amd64_linux_lam_untag_mask and linux_process_address_in_memtag_page
> may now return earlier in case the file is empty.
>
> Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Reviewed-By: Christina Joos <christina.joos@intel.com>
> ---
>  gdb/amd64-linux-tdep.c |  12 ++--
>  gdb/linux-tdep.c       | 122 ++++++++++++++++++-----------------------
>  gdb/sparc64-tdep.c     |  15 +++--
>  gdb/target.h           |  78 ++++++++++++++++++++++++++
>  4 files changed, 143 insertions(+), 84 deletions(-)
>
> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> index 9b23db72bbe..52f16c953d1 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -1848,14 +1848,11 @@ amd64_linux_lam_untag_mask ()
>    if (inf->fake_pid_p)
>      return DEFAULT_TAG_MASK;
>  
> -  const std::string filename = string_printf ("/proc/%d/status", inf->pid);
> -  gdb::unique_xmalloc_ptr<char> status_file
> -    = target_fileio_read_stralloc (nullptr, filename.c_str ());
> -
> -  if (status_file == nullptr)
> +  file_reader_t<char> proc_status (string_printf ("/proc/%d/status", inf->pid));
> +  if (!proc_status)
>      return DEFAULT_TAG_MASK;
>  
> -  std::string_view status_file_view (status_file.get ());
> +  std::string_view status_file_view (proc_status.data ());
>    constexpr std::string_view untag_mask_str = "untag_mask:\t";
>    const size_t found = status_file_view.find (untag_mask_str);
>    if (found != std::string::npos)
> @@ -1867,7 +1864,8 @@ amd64_linux_lam_untag_mask ()
>        unsigned long long result = std::strtoul (start, &endptr, 0);
>        if (errno != 0 || endptr == start)
>  	error (_("Failed to parse untag_mask from file %ps."),
> -	       styled_string (file_name_style.style (), filename.c_str ()));
> +	       styled_string (file_name_style.style (),
> +			      proc_status.c_filepath ()));
>  
>        return result;
>      }
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index 588984a1ca4..84614bc91a0 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -1551,7 +1551,7 @@ parse_smaps_key_value (const char *keyword, const char *line,
>     DATA is the contents of the smaps file.  The parsed contents are stored
>     into the SMAPS vector.  */
>  
> -static std::vector<struct smaps_data>
> +static std::vector<smaps_data>

Throughout this patch there's a bunch of places where you've done
nothing but delete the 'struct' prefix.  It's OK to do this in code that
you're touching anyway as part of this patch, but any, like this, that
are in code that you'd not otherwise touch, are unrelated changes and
should be moved into a separate patch.

I think you should either drop these, or have a first patch which does a
"remove some struct prefixes" cleanup, your choice.


> @@ -2346,27 +2343,24 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
>    p->pr_pid = ptid.pid ();
>  
>    /* Copying the program name.  Only the basename matters.  */
> -  basename = lbasename (fname.get ());
> +  basename = lbasename (cmdline.data ());
>    strncpy (p->pr_fname, basename, sizeof (p->pr_fname) - 1);
>    p->pr_fname[sizeof (p->pr_fname) - 1] = '\0';
>  
>    const std::string &infargs = current_inferior ()->args ();
>  
>    /* The arguments of the program.  */
> -  std::string psargs = fname.get ();
> +  std::string psargs = cmdline.data ();
>    if (!infargs.empty ())
>      psargs += ' ' + infargs;
>  
>    strncpy (p->pr_psargs, psargs.c_str (), sizeof (p->pr_psargs) - 1);
>    p->pr_psargs[sizeof (p->pr_psargs) - 1] = '\0';
>  
> -  xsnprintf (filename, sizeof (filename), "/proc/%ld/stat", ptid.lwp ());
> -  /* The contents of `/proc/PID/stat'.  */
> -  gdb::unique_xmalloc_ptr<char> proc_stat_contents
> -    = target_fileio_read_stralloc (NULL, filename);
> -  char *proc_stat = proc_stat_contents.get ();
> -
> -  if (proc_stat == NULL || *proc_stat == '\0')
> +  file_reader_t<char> stat_freader
> +    (string_printf ("/proc/%ld/stat", ptid.lwp ()));
> +  const char *proc_stat = stat_freader.data ();
> +  if (!stat_freader || *proc_stat == '\0')

We access the data here before checking if the read was successful.
This works fine, but doesn't seem ideal.  Later on I suggest that maybe
file_reader_t::data should assert that we're no in the error state, and
this is what I was looking at when I started thinking about that.

If you really think we should support reading data when in an error
state, then the data method should document what the return value is
when the file_reader_t is in the error state.



> diff --git a/gdb/target.h b/gdb/target.h
> index 819279c08fc..017918b6582 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -2341,6 +2341,84 @@ extern LONGEST target_fileio_read_alloc (struct inferior *inf,
>  extern gdb::unique_xmalloc_ptr<char> target_fileio_read_stralloc
>      (struct inferior *inf, const char *filename, LONGEST *len = nullptr);
>  
> +/* Helper class for reading the content of a file on the target.  */
> +template <typename T>
> +class file_reader_t

I'm pretty sure that types ending with _t are reserved by ... some
spec.  We should avoid this and ideally, pick a name that better
describes what the class does, e.g. target_file_reader.

> +{
> +  /* The filepath of the file being read.  */
> +  std::string m_filepath;

The ship has mostly sailed already, but "path" should be used for lists
of locations, like the $PATH variable.  It would be better to just use
filename, m_filename, etc.  I am fully aware that 'path' is used
throughout GDB in place of filename, but we might as well avoid adding
another here.

This should be fixed throughout this class.

> +  /* Smart pointer to the data.  */
> +  gdb::unique_xmalloc_ptr<T> m_data;
> +  /* Number of bytes read.  */
> +  LONGEST m_size;

GDB style usually puts a space between member variables, e.g.:

  /* The filepath of the file being read.  */
  std::string m_filepath;

  /* Smart pointer to the data.  */
  gdb::unique_xmalloc_ptr<T> m_data;

  /* Number of bytes read.  */
  LONGEST m_size;

> +
> +public:
> +  file_reader_t (const std::string &filepath)
> +    : m_filepath (filepath)
> +    , m_size (0)
> +  {
> +    if constexpr (std::is_same_v<T, char>)
> +      m_data = target_fileio_read_stralloc (nullptr, m_filepath.c_str (),
> +					    &m_size);
> +    else
> +      {
> +	gdb_byte *buf = nullptr;
> +	m_size = target_fileio_read_alloc (nullptr, m_filepath.c_str (), &buf);
> +	m_data = gdb::unique_xmalloc_ptr<T> (reinterpret_cast<T *>(buf));
> +      }
> +  }

There's a bug hiding in here when T is not 'char'.  If the file being
read is empty then target_fileio_read_alloc returns 0 but leaves *BUF
unchanged, i.e. as nullptr.

Given that, despite successfully reading the empty file, empty() will
return false and error() will return true.

Also, given this is being written as a general helper class, it might be
a good idea to define how the inferior is passed in, rather than leaving
that for future users to do.

> +
> +  file_reader_t (file_reader_t &&) = default;
> +  file_reader_t &operator= (file_reader_t &&) = default;
> +
> +  DISABLE_COPY_AND_ASSIGN (file_reader_t);
> +
> +  /* Return true if the file was read successfully but contained no data.  */
> +  bool empty () const noexcept
> +  { return m_data != nullptr && m_size == 0; }
> +
> +  /* Return true if the file could not be read.  */
> +  bool error () const noexcept
> +  { return m_data == nullptr || m_size < 0; }
> +
> +  /* Return true if the file was read successfully and is non-empty.  */
> +  explicit operator bool () const noexcept
> +  { return !(error () || empty ()); }

I'm really not a fan of this API.  Consider this code from earlier in
this patch:

  file_reader_t<char> proc_status (string_printf ("/proc/%d/status", inf->pid));
  if (!proc_status)
    return DEFAULT_TAG_MASK;

I don't think it's obvious that !proc_status means error or empty.  I
think a much less error prone API would be to just add a new member
function:

  bool empty_or_error () const noexcept
  {  return this->empty () || this->error (); }

And then use that.  It's more typing for sure, but it's also crystal
clear what's going on.

> +
> +  /* Return a pointer to the data.  */
> +  T *data () const noexcept
> +  { return m_data.get (); }

Might be a good idea to assert that we're not in the error state.

> +
> +  /* Return the number of bytes read.  */
> +  LONGEST size () const noexcept
> +  {
> +    /* For char buffers, size() corresponds to the size of the read data. Some
> +       null-terminator characters are possibly scattered throughout the data.
> +       Consequently, strlen() might not reflect the actual size.  */
> +    return m_size;
> +  }

Again, maybe assert that we're not in the error state.  I think there's
only one user of this right now, and it already checks for errors before
calling size.

> +
> +  /* Return a span of the data.  */
> +  gdb::array_view<T> view () const noexcept
> +  { return gdb::array_view<T> (m_data.get (), size ()); }
> +
> +  /* Return a span of the data, reinterpreted as U objects.  */
> +  template <typename U>
> +  gdb::array_view<U> cast_view () const noexcept
> +  {
> +    return gdb::array_view<U> (reinterpret_cast<U *> (m_data.get ()),
> +			       size () * sizeof (T) / sizeof (U));
> +  }

It would be a good idea to say in the comment what happens if the file
size is not a multiple of 'sizeof (U)'.  Or do we even want to support
this case?

Thanks,
Andrew
diff mbox series

Patch

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 9b23db72bbe..52f16c953d1 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1848,14 +1848,11 @@  amd64_linux_lam_untag_mask ()
   if (inf->fake_pid_p)
     return DEFAULT_TAG_MASK;
 
-  const std::string filename = string_printf ("/proc/%d/status", inf->pid);
-  gdb::unique_xmalloc_ptr<char> status_file
-    = target_fileio_read_stralloc (nullptr, filename.c_str ());
-
-  if (status_file == nullptr)
+  file_reader_t<char> proc_status (string_printf ("/proc/%d/status", inf->pid));
+  if (!proc_status)
     return DEFAULT_TAG_MASK;
 
-  std::string_view status_file_view (status_file.get ());
+  std::string_view status_file_view (proc_status.data ());
   constexpr std::string_view untag_mask_str = "untag_mask:\t";
   const size_t found = status_file_view.find (untag_mask_str);
   if (found != std::string::npos)
@@ -1867,7 +1864,8 @@  amd64_linux_lam_untag_mask ()
       unsigned long long result = std::strtoul (start, &endptr, 0);
       if (errno != 0 || endptr == start)
 	error (_("Failed to parse untag_mask from file %ps."),
-	       styled_string (file_name_style.style (), filename.c_str ()));
+	       styled_string (file_name_style.style (),
+			      proc_status.c_filepath ()));
 
       return result;
     }
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 588984a1ca4..84614bc91a0 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1551,7 +1551,7 @@  parse_smaps_key_value (const char *keyword, const char *line,
    DATA is the contents of the smaps file.  The parsed contents are stored
    into the SMAPS vector.  */
 
-static std::vector<struct smaps_data>
+static std::vector<smaps_data>
 parse_smaps_data (const char *data,
 		  const std::string &maps_filename)
 {
@@ -1561,7 +1561,7 @@  parse_smaps_data (const char *data,
 
   line = strtok_r ((char *) data, "\n", &t);
 
-  std::vector<struct smaps_data> smaps;
+  std::vector<smaps_data> smaps;
 
   while (line != NULL)
     {
@@ -1673,7 +1673,7 @@  parse_smaps_data (const char *data,
 	    }
 	}
       /* Save the smaps entry to the vector.  */
-	struct smaps_data map;
+	smaps_data map;
 
 	map.start_address = m.addr;
 	map.end_address = m.endaddr;
@@ -1697,6 +1697,12 @@  parse_smaps_data (const char *data,
   return smaps;
 }
 
+static std::vector<smaps_data>
+parse_smaps_data (const file_reader_t<char> &freader)
+{
+  return parse_smaps_data (freader.data (), freader.filepath ());
+}
+
 /* Helper that checks if an address is in a memory tag page for a live
    process.  */
 
@@ -1708,17 +1714,13 @@  linux_process_address_in_memtag_page (CORE_ADDR address)
 
   ptid_t ptid = get_process_reference_ptid ();
 
-  std::string smaps_file = string_printf ("/proc/%ld/smaps", ptid.lwp ());
-
-  gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (NULL, smaps_file.c_str ());
-
-  if (data == nullptr)
+  file_reader_t<char> smaps_freader
+    (string_printf ("/proc/%ld/smaps", ptid.lwp ()));
+  if (!smaps_freader)
     return false;
 
   /* Parse the contents of smaps into a vector.  */
-  std::vector<struct smaps_data> smaps
-    = parse_smaps_data (data.get (), smaps_file);
+  std::vector<smaps_data> smaps = parse_smaps_data (smaps_freader);
 
   for (const smaps_data &map : smaps)
     {
@@ -1782,17 +1784,13 @@  linux_find_memory_regions_full (struct gdbarch *gdbarch,
 
   if (use_coredump_filter)
     {
-      std::string core_dump_filter_name
-	= string_printf ("/proc/%ld/coredump_filter", ptid.lwp ());
-
-      gdb::unique_xmalloc_ptr<char> coredumpfilterdata
-	= target_fileio_read_stralloc (NULL, core_dump_filter_name.c_str ());
-
-      if (coredumpfilterdata != NULL)
+      file_reader_t<char> coredump_filter_freader
+	(string_printf ("/proc/%ld/coredump_filter", ptid.lwp ()));
+      if (coredump_filter_freader)
 	{
 	  unsigned int flags;
 
-	  sscanf (coredumpfilterdata.get (), "%x", &flags);
+	  sscanf (coredump_filter_freader.data (), "%x", &flags);
 	  filterflags = (enum filter_flag) flags;
 	}
     }
@@ -1813,10 +1811,9 @@  linux_find_memory_regions_full (struct gdbarch *gdbarch,
     }
 
   /* Parse the contents of smaps into a vector.  */
-  std::vector<struct smaps_data> smaps
-    = parse_smaps_data (data.get (), maps_filename);
+  std::vector<smaps_data> smaps = parse_smaps_data (data.get (), maps_filename);
 
-  for (const struct smaps_data &map : smaps)
+  for (const smaps_data &map: smaps)
     {
       /* Invoke the callback function to create the corefile segment.  */
       if (should_dump_mapping_p (filterflags, map))
@@ -2295,9 +2292,6 @@  linux_corefile_parse_exec_context (struct gdbarch *gdbarch, bfd *cbfd)
 static bool
 linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
 {
-  /* The filename which we will use to obtain some info about the process.
-     We will basically use this to store the `/proc/PID/FILENAME' file.  */
-  char filename[100];
   /* The basename of the executable.  */
   const char *basename;
   /* Temporary buffer.  */
@@ -2319,24 +2313,27 @@  linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
 
   gdb_assert (p != nullptr);
 
-  /* Obtaining PID and filename.  */
-  xsnprintf (filename, sizeof (filename), "/proc/%ld/cmdline", ptid.lwp ());
-  /* The full name of the program which generated the corefile.  */
-  gdb_byte *buf = nullptr;
-  LONGEST buf_len = target_fileio_read_alloc (nullptr, filename, &buf);
-  gdb::unique_xmalloc_ptr<char> fname ((char *)buf);
+  file_reader_t<gdb_byte> cmdline_freader
+    (string_printf ("/proc/%ld/cmdline", ptid.lwp ()));
+  if (!cmdline_freader)
+    return false;
 
-  if (buf_len < 1 || fname.get () == nullptr || fname.get ()[0] == '\0')
+  /* /proc/<pid>/cmdline stores the command-line arguments as a sequence of
+     NUL-separated strings.  */
+  gdb::array_view<char> cmdline = cmdline_freader.cast_view<char> ();
+  /* The buffer points to the full name of the program which generated the
+     corefile.  */
+  if (cmdline.size () < 1 || cmdline[0] == '\0')
     {
       /* No program name was read, so we won't be able to retrieve more
 	 information about the process.  */
       return false;
     }
-  if (fname.get ()[buf_len - 1] != '\0')
+  if (cmdline[cmdline.size () - 1] != '\0')
     {
       warning (_("target file %s "
 		 "does not contain a trailing null character"),
-	       filename);
+	       cmdline_freader.c_filepath ());
       return false;
     }
 
@@ -2346,27 +2343,24 @@  linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
   p->pr_pid = ptid.pid ();
 
   /* Copying the program name.  Only the basename matters.  */
-  basename = lbasename (fname.get ());
+  basename = lbasename (cmdline.data ());
   strncpy (p->pr_fname, basename, sizeof (p->pr_fname) - 1);
   p->pr_fname[sizeof (p->pr_fname) - 1] = '\0';
 
   const std::string &infargs = current_inferior ()->args ();
 
   /* The arguments of the program.  */
-  std::string psargs = fname.get ();
+  std::string psargs = cmdline.data ();
   if (!infargs.empty ())
     psargs += ' ' + infargs;
 
   strncpy (p->pr_psargs, psargs.c_str (), sizeof (p->pr_psargs) - 1);
   p->pr_psargs[sizeof (p->pr_psargs) - 1] = '\0';
 
-  xsnprintf (filename, sizeof (filename), "/proc/%ld/stat", ptid.lwp ());
-  /* The contents of `/proc/PID/stat'.  */
-  gdb::unique_xmalloc_ptr<char> proc_stat_contents
-    = target_fileio_read_stralloc (NULL, filename);
-  char *proc_stat = proc_stat_contents.get ();
-
-  if (proc_stat == NULL || *proc_stat == '\0')
+  file_reader_t<char> stat_freader
+    (string_printf ("/proc/%ld/stat", ptid.lwp ()));
+  const char *proc_stat = stat_freader.data ();
+  if (!stat_freader || *proc_stat == '\0')
     {
       /* Despite being unable to read more information about the
 	 process, we return true here because at least we have its
@@ -2438,13 +2432,10 @@  linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
 
   /* Finally, obtaining the UID and GID.  For that, we read and parse the
      contents of the `/proc/PID/status' file.  */
-  xsnprintf (filename, sizeof (filename), "/proc/%ld/status", ptid.lwp ());
-  /* The contents of `/proc/PID/status'.  */
-  gdb::unique_xmalloc_ptr<char> proc_status_contents
-    = target_fileio_read_stralloc (NULL, filename);
-  char *proc_status = proc_status_contents.get ();
-
-  if (proc_status == NULL || *proc_status == '\0')
+  file_reader_t<char> status_freader
+    (string_printf ("/proc/%ld/status", ptid.lwp ()));
+  char *proc_status = status_freader.data ();
+  if (!status_freader || *proc_status == '\0')
     {
       /* Returning true since we already have a bunch of information.  */
       return true;
@@ -2837,9 +2828,6 @@  linux_gdb_signal_to_target (struct gdbarch *gdbarch,
 static bool
 linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
 {
-  char filename[100];
-  long pid;
-
   if (target_auxv_search (AT_SYSINFO_EHDR, &range->start) <= 0)
     return false;
 
@@ -2877,7 +2865,7 @@  linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
   if (current_inferior ()->fake_pid_p)
     return false;
 
-  pid = current_inferior ()->pid;
+  long pid = current_inferior ()->pid;
 
   /* Note that reading /proc/PID/task/PID/maps (1) is much faster than
      reading /proc/PID/maps (2).  The later identifies thread stacks
@@ -2887,15 +2875,14 @@  linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
      a few thousand threads, (1) takes a few milliseconds, while (2)
      takes several seconds.  Also note that "smaps", what we read for
      determining core dump mappings, is even slower than "maps".  */
-  xsnprintf (filename, sizeof filename, "/proc/%ld/task/%ld/maps", pid, pid);
-  gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (NULL, filename);
-  if (data != NULL)
+  file_reader_t<char> task_maps_freader
+    (string_printf ("/proc/%ld/task/%ld/maps", pid, pid));
+  if (task_maps_freader)
     {
       char *line;
       char *saveptr = NULL;
 
-      for (line = strtok_r (data.get (), "\n", &saveptr);
+      for (line = strtok_r (task_maps_freader.data (), "\n", &saveptr);
 	   line != NULL;
 	   line = strtok_r (NULL, "\n", &saveptr))
 	{
@@ -2913,8 +2900,9 @@  linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
 	    }
 	}
     }
-  else
-    warning (_("unable to open /proc file '%s'"), filename);
+  else if (task_maps_freader.error ())
+    warning (_("unable to open /proc file '%s'"),
+	     task_maps_freader.c_filepath ());
 
   return false;
 }
@@ -3241,16 +3229,12 @@  linux_address_in_shadow_stack_mem_range
 
   ptid_t ptid = get_process_reference_ptid ();
 
-  std::string smaps_file = string_printf ("/proc/%ld/smaps", ptid.lwp ());
-
-  gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (nullptr, smaps_file.c_str ());
-
-  if (data == nullptr)
+  file_reader_t<char> smaps_freader
+    (string_printf ("/proc/%ld/smaps", ptid.lwp ()));
+  if (!smaps_freader)
     return false;
 
-  const std::vector<smaps_data> smaps
-    = parse_smaps_data (data.get (), smaps_file);
+  const std::vector<smaps_data> smaps = parse_smaps_data (smaps_freader);
 
   auto find_addr_mem_range = [&addr] (const smaps_data &map)
     {
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 93db3417a2a..97f8fa72e52 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -301,18 +301,16 @@  adi_tag_fd ()
 static bool
 adi_is_addr_mapped (CORE_ADDR vaddr, size_t cnt)
 {
-  char filename[MAX_PROC_NAME_SIZE];
   size_t i = 0;
 
   pid_t pid = inferior_ptid.pid ();
-  snprintf (filename, sizeof filename, "/proc/%ld/adi/maps", (long) pid);
-  gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (NULL, filename);
-  if (data)
+  file_reader_t<char> adi_maps_freader
+    (string_printf ("/proc/%d/adi/maps", pid));
+  if (adi_maps_freader)
     {
       adi_stat_t adi_stat = get_adi_info (pid);
       char *saveptr;
-      for (char *line = strtok_r (data.get (), "\n", &saveptr);
+      for (char *line = strtok_r (adi_maps_freader.data (), "\n", &saveptr);
 	   line;
 	   line = strtok_r (NULL, "\n", &saveptr))
 	{
@@ -328,8 +326,9 @@  adi_is_addr_mapped (CORE_ADDR vaddr, size_t cnt)
 	    }
 	}
       }
-  else
-    warning (_("unable to open /proc file '%s'"), filename);
+  else if (adi_maps_freader.error ())
+    warning (_("unable to open /proc file '%s'"),
+	     adi_maps_freader.c_filepath ());
 
   return false;
 }
diff --git a/gdb/target.h b/gdb/target.h
index 819279c08fc..017918b6582 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2341,6 +2341,84 @@  extern LONGEST target_fileio_read_alloc (struct inferior *inf,
 extern gdb::unique_xmalloc_ptr<char> target_fileio_read_stralloc
     (struct inferior *inf, const char *filename, LONGEST *len = nullptr);
 
+/* Helper class for reading the content of a file on the target.  */
+template <typename T>
+class file_reader_t
+{
+  /* The filepath of the file being read.  */
+  std::string m_filepath;
+  /* Smart pointer to the data.  */
+  gdb::unique_xmalloc_ptr<T> m_data;
+  /* Number of bytes read.  */
+  LONGEST m_size;
+
+public:
+  file_reader_t (const std::string &filepath)
+    : m_filepath (filepath)
+    , m_size (0)
+  {
+    if constexpr (std::is_same_v<T, char>)
+      m_data = target_fileio_read_stralloc (nullptr, m_filepath.c_str (),
+					    &m_size);
+    else
+      {
+	gdb_byte *buf = nullptr;
+	m_size = target_fileio_read_alloc (nullptr, m_filepath.c_str (), &buf);
+	m_data = gdb::unique_xmalloc_ptr<T> (reinterpret_cast<T *>(buf));
+      }
+  }
+
+  file_reader_t (file_reader_t &&) = default;
+  file_reader_t &operator= (file_reader_t &&) = default;
+
+  DISABLE_COPY_AND_ASSIGN (file_reader_t);
+
+  /* Return true if the file was read successfully but contained no data.  */
+  bool empty () const noexcept
+  { return m_data != nullptr && m_size == 0; }
+
+  /* Return true if the file could not be read.  */
+  bool error () const noexcept
+  { return m_data == nullptr || m_size < 0; }
+
+  /* Return true if the file was read successfully and is non-empty.  */
+  explicit operator bool () const noexcept
+  { return !(error () || empty ()); }
+
+  /* Return a pointer to the data.  */
+  T *data () const noexcept
+  { return m_data.get (); }
+
+  /* Return the number of bytes read.  */
+  LONGEST size () const noexcept
+  {
+    /* For char buffers, size() corresponds to the size of the read data. Some
+       null-terminator characters are possibly scattered throughout the data.
+       Consequently, strlen() might not reflect the actual size.  */
+    return m_size;
+  }
+
+  /* Return a span of the data.  */
+  gdb::array_view<T> view () const noexcept
+  { return gdb::array_view<T> (m_data.get (), size ()); }
+
+  /* Return a span of the data, reinterpreted as U objects.  */
+  template <typename U>
+  gdb::array_view<U> cast_view () const noexcept
+  {
+    return gdb::array_view<U> (reinterpret_cast<U *> (m_data.get ()),
+			       size () * sizeof (T) / sizeof (U));
+  }
+
+  /* Return the path of the file that was read.  */
+  const std::string &filepath () const noexcept
+  { return m_filepath; }
+
+  /* Return the path of the file that was read as a C string.  */
+  const char *c_filepath () const noexcept
+  { return m_filepath.c_str (); }
+};
+
 /* Invalidate the target associated with open handles that were open
    on target TARG, since we're about to close (and maybe destroy) the
    target.  The handles remain open from the client's perspective, but