[PATCH v1 05/10] gdb: introduce helper class file_reader_t

[Matthieu Longo <matthieu.longo@arm.com>]

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.
---
 gdb/amd64-linux-tdep.c |  12 ++---
 gdb/linux-tdep.c       | 102 +++++++++++++++++------------------------
 gdb/sparc64-tdep.c     |  12 ++---
 gdb/target.h           |  65 ++++++++++++++++++++++++++
 4 files changed, 117 insertions(+), 74 deletions(-)

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index a5ac26654cf..e7841917eaf 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1851,14 +1851,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)
@@ -1870,7 +1867,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 b89f4ee0717..a12a69f03a2 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1661,6 +1661,12 @@ parse_smaps_data (const char *data,
   return smaps;
 }
 
+static std::vector<struct 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.  */
 
@@ -1672,17 +1678,13 @@ linux_process_address_in_memtag_page (CORE_ADDR address)
 
   ptid_t ptid = current_inferior ()->first_alive_thread ();
 
-  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<struct smaps_data> smaps = parse_smaps_data (smaps_freader);
 
   for (const smaps_data &map : smaps)
     {
@@ -1746,17 +1748,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;
 	}
     }
@@ -2259,9 +2257,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.  */
@@ -2285,23 +2280,24 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
 
   /* Obtaining PID and filename.  */
   pid = inferior_ptid.pid ();
-  xsnprintf (filename, sizeof (filename), "/proc/%d/cmdline", (int) pid);
-  /* 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/%d/cmdline", pid));
+  if (!cmdline_freader)
+    return false;
 
-  if (buf_len < 1 || fname.get () == nullptr || fname.get ()[0] == '\0')
+  /* The full name of the program which generated the corefile.  */
+  gdb::array_view<char> cmdline = cmdline_freader.cast_view<char> ();
+  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;
     }
 
@@ -2311,27 +2307,23 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
   p->pr_pid = 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/%d/stat", (int) pid);
-  /* 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/%d/stat", pid));
+  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
@@ -2403,13 +2395,9 @@ 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/%d/status", (int) pid);
-  /* 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/%d/status", pid));
+  char *proc_status = status_freader.data ();
+  if (!status_freader || *proc_status == '\0')
     {
       /* Returning true since we already have a bunch of information.  */
       return true;
@@ -2802,9 +2790,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;
 
@@ -2842,7 +2827,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
@@ -2852,15 +2837,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))
 	{
@@ -2879,7 +2863,8 @@ linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
 	}
     }
   else
-    warning (_("unable to open /proc file '%s'"), filename);
+    warning (_("unable to open /proc file '%s'"),
+	     task_maps_freader.c_filepath ());
 
   return false;
 }
@@ -3168,16 +3153,11 @@ linux_address_in_shadow_stack_mem_range
 
   const int pid = current_inferior ()->pid;
 
-  std::string smaps_file = string_printf ("/proc/%d/smaps", pid);
-
-  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/%d/smaps", pid));
+  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..811b76f27d8 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -305,14 +305,13 @@ adi_is_addr_mapped (CORE_ADDR vaddr, size_t cnt)
   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))
 	{
@@ -329,7 +328,8 @@ adi_is_addr_mapped (CORE_ADDR vaddr, size_t cnt)
 	}
       }
   else
-    warning (_("unable to open /proc file '%s'"), filename);
+    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 4215553033c..d6fc101f205 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2338,6 +2338,71 @@ 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, size_t *len = nullptr);
 
+/* Helper class for reading a file.  */
+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;
+  /* Size of the data.  */
+  size_t 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));
+      }
+  }
+
+  /* Return true if the file was read successfully.  */
+  operator bool () const noexcept
+  { return m_data != nullptr && m_size > 0; }
+
+  /* Return a pointer to the data.  */
+  T *data () const noexcept
+  { return m_data.get (); }
+
+  /* Return the size of the data.  */
+  size_t 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, cast to a different type.  */
+  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 filepath of the file that was read.  */
+  const std::string &filepath () const noexcept
+  { return m_filepath; }
+
+  /* Return the filepath 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
-- 
2.55.0