[PATCH v3 0/8] gdb: introduce file_reader_t to read procfs files

[PATCH v3 0/8] gdb: introduce file_reader_t to read procfs files

[Matthieu Longo]

This series introduces target_file_reader, a helper class for reading target files, and migrates existing /proc file handling to use it. It also refactors the /proc mapping parser to operate on bounded views rather than NUL-terminated strings.

The series is organized as follows:
- Patch 1 adds gdb::ranges::replace, a C++17-compatible replacement for std::ranges::replace.
- Patches 2 and 3 contain small cleanups identified during review: removing redundant struct prefixes and styling /proc filenames in warning messages.
- Patch 4 adds extract_next_field, a helper for extracting separator-delimited views from a buffer.
- Patch 5 refactors /proc mapping parsing to use gdb::array_view<char> and extract_view_from_buffer, removing the dependency on NUL-terminated strings and strtok_r.
- Patch 6 introduces target_file_reader, which encapsulates the contents and path of a target file and provides convenient accessors and typed views.
- Patches 7 and 8 migrate existing procfs parsing to target_file_reader and remove the now-obsolete parse_smaps_data interface.

== Review status ==
Most of the patches have been already reviewed once by Andrew Burgess in v2, and others reviewers in v1.
However, for clarity:
- Patches 2 and 3 are new but based on comments from Andrew Burgess in v2.
- Patch 5 contains new or substantially changed code and require more attention. This rework was triggered by the discovery of a bug in patch 4/6 in v2.


== Changes since v2 ==

v2: https://inbox.sourceware.org/gdb-patches/20260825100912.514232-1-matthieu.longo@arm.com/
- Renamed extract_view_from_buffer to extract_next_field, and add unit tests in selftests.
- Refactored /proc mapping parsing to operate on gdb::array_view<char> instead of NUL-terminated strings. read_mapping now parses bounded ranges and diagnoses malformed mapping fields.
- Split the cleanup removing redundant struct prefixes from smaps_data, as suggested by Christina Joos and Andrew Burgess.
- Added filename styling to /proc warning messages, as suggested by Andrew Burgess.


This series depends on: https://inbox.sourceware.org/gdb-patches/20260917201755.589524-1-simon.marchi@efficios.com/

Regards,
Matthieu


Matthieu Longo (8):
  gdb support: add gdb::ranges::replace algorithm
  gdb/linux: remove redundant struct prefixes from smaps_data
  gdb/linux: style filenames in /proc warning messages
  gdbsupport: add extract_next_field helper
  gdb/linux: refactor /proc mapping parsing to use array_view
  gdb: introduce helper class target_file_reader
  gdb/linux-tdep: use target_file_reader for procfs parsing
  gdb/linux-tdep: remove legacy parse_smaps_data overload

 gdb/Makefile.in                        |   1 +
 gdb/amd64-linux-tdep.c                 |  12 +-
 gdb/linux-tdep.c                       | 406 +++++++++++++------------
 gdb/sparc64-tdep.c                     |  17 +-
 gdb/target.h                           | 116 +++++++
 gdb/unittests/common-utils-selftests.c |  32 ++
 gdb/unittests/ranges-selftests.c       |  63 ++++
 gdbsupport/common-utils.h              |  50 +++
 gdbsupport/ranges.h                    |  39 +++
 9 files changed, 530 insertions(+), 206 deletions(-)
 create mode 100644 gdb/unittests/ranges-selftests.c
 create mode 100644 gdbsupport/ranges.h

-- 
2.55.0

[PATCH v3 1/8] gdb support: add gdb::ranges::replace algorithm

[Matthieu Longo]

Provide a C++17-compatible replacement for the C++20 std::ranges::replace
algorithm, allowing callers to use a consistent interface until GDB
transitions to C++20. The helper should be removed once the C++ standard
library implementation become available.

Also add unit tests for ranges::replace.

https://en.cppreference.com/cpp/algorithm/ranges/replace
---
 gdb/Makefile.in                  |  1 +
 gdb/unittests/ranges-selftests.c | 63 ++++++++++++++++++++++++++++++++
 gdbsupport/ranges.h              | 39 ++++++++++++++++++++
 3 files changed, 103 insertions(+)
 create mode 100644 gdb/unittests/ranges-selftests.c
 create mode 100644 gdbsupport/ranges.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index d1574ec2d2c..9d34b6cd3e4 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -483,6 +483,7 @@ SELFTESTS_SRCS = \
 	unittests/ptid-selftests.c \
 	unittests/main-thread-selftests.c \
 	unittests/mkdir-recursive-selftests.c \
+	unittests/ranges-selftests.c \
 	unittests/remote-arg-selftests.c \
 	unittests/rsp-low-selftests.c \
 	unittests/scoped_fd-selftests.c \
diff --git a/gdb/unittests/ranges-selftests.c b/gdb/unittests/ranges-selftests.c
new file mode 100644
index 00000000000..6791502cc1f
--- /dev/null
+++ b/gdb/unittests/ranges-selftests.c
@@ -0,0 +1,63 @@
+/* Self tests for gdb::ranges algorithms for GDB, the GNU debugger.
+
+   Copyright (C) 2017-2026 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "gdbsupport/selftest.h"
+#include "gdbsupport/array-view.h"
+#include "gdbsupport/ranges.h"
+#include <array>
+#include <vector>
+
+namespace selftests {
+namespace ranges_replace_tests {
+
+/* Entry point.  */
+
+static void
+run_tests ()
+{
+  /* With owning container, no copy and elements correctly replaced.  */
+  {
+    std::vector<int> vec = {1, 2, 3, 2};
+    gdb::ranges::replace (vec, 2, 1);
+    SELF_CHECK (vec[0] == 1);
+    SELF_CHECK (vec[1] == 1);
+    SELF_CHECK (vec[2] == 3);
+    SELF_CHECK (vec[3] == 1);
+  }
+
+  /* With non-owning container.  */
+  {
+    std::array<int, 4> array = {1, 2, 3, 2};
+    gdb::array_view<int> view = array;
+    gdb::ranges::replace (view, 2, 1);
+    SELF_CHECK (array[0] == 1);
+    SELF_CHECK (array[1] == 1);
+    SELF_CHECK (array[2] == 3);
+    SELF_CHECK (array[3] == 1);
+  }
+}
+
+} /* namespace ranges_replaceq_tests */
+} /* namespace selftests */
+
+INIT_GDB_FILE (ranges_selftests)
+{
+  selftests::register_test ("ranges_replace",
+			    selftests::ranges_replace_tests::run_tests);
+}
diff --git a/gdbsupport/ranges.h b/gdbsupport/ranges.h
new file mode 100644
index 00000000000..96d9c0162bb
--- /dev/null
+++ b/gdbsupport/ranges.h
@@ -0,0 +1,39 @@
+/* Copyright (C) 2026 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_RANGES_H
+#define GDBSUPPORT_RANGES_H
+
+namespace gdb {
+namespace ranges {
+
+/* Replace all occurrences of a value in the provided range.
+
+   Note: this helper is a reimplementation of std::ranges::replace, only
+   available from C++20 onwards, and consequently, should be replaced by
+   std::ranges::replace once GDB switches to C++20.  */
+
+template <class Range, typename T>
+void replace (Range &&r, const T &old_value, const T &new_value)
+{
+  std::replace (r.begin (), r.end (), old_value, new_value);
+}
+
+} /* namespace ranges */
+} /* namespace gdb */
+
+#endif /* GDBSUPPORT_RANGES_H */
-- 
2.55.0

[PATCH v3 2/8] gdb/linux: remove redundant struct prefixes from smaps_data

[Matthieu Longo]

Remove the redundant 'struct' prefix from uses of smaps_data in
linux-tdep.c. In C++, the structure name can be used directly as
a type name.

Suggested-By: Christina Joos <christina.joos@intel.com>
---
 gdb/linux-tdep.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 4f6910694d5..799117a6d1b 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;
@@ -1716,7 +1716,7 @@ linux_process_address_in_memtag_page (CORE_ADDR address)
     return false;
 
   /* Parse the contents of smaps into a vector.  */
-  std::vector<struct smaps_data> smaps
+  std::vector<smaps_data> smaps
     = parse_smaps_data (data.get (), smaps_file);
 
   for (const smaps_data &map : smaps)
@@ -1812,10 +1812,10 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
     }
 
   /* Parse the contents of smaps into a vector.  */
-  std::vector<struct smaps_data> smaps
+  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))
-- 
2.55.0

[PATCH v3 3/8] gdb/linux: style filenames in /proc warning messages

[Matthieu Longo]

Use filename styling for the paths printed by warning messages in
linux_info_proc and linux_vsyscall_range_raw.

Replace the plain '%s' formatting with '%ps' and styled_string using
file_name_style, making these warnings consistent with other diagnostics
that display filenames.

Suggested-By: Andrew Burgess <aburgess@redhat.com>
---
 gdb/linux-tdep.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 799117a6d1b..bd4c1d5540f 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -932,7 +932,8 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
 	  gdb_printf ("cmdline = '%s'\n", buffer);
 	}
       else
-	warning (_("unable to open /proc file '%s'"), filename);
+	warning (_("unable to open /proc file '%ps'"),
+		 styled_string (file_name_style.style (), filename));
     }
   if (cwd_f)
     {
@@ -942,7 +943,8 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
       if (contents.has_value ())
 	gdb_printf ("cwd = '%s'\n", contents->c_str ());
       else
-	warning (_("unable to read link '%s'"), filename);
+	warning (_("unable to read link '%ps'"),
+		 styled_string (file_name_style.style (), filename));
     }
   if (environ_f)
     {
@@ -966,7 +968,8 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
 	    }
 	}
       else
-	warning (_("unable to open /proc file '%s'"), filename);
+	warning (_("unable to open /proc file '%ps'"),
+		 styled_string (file_name_style.style (), filename));
     }
   if (exe_f)
     {
@@ -976,7 +979,8 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
       if (contents.has_value ())
 	gdb_printf ("exe = '%s'\n", contents->c_str ());
       else
-	warning (_("unable to read link '%s'"), filename);
+	warning (_("unable to read link '%ps'"),
+		 styled_string (file_name_style.style (), filename));
     }
   if (mappings_f)
     {
@@ -1021,7 +1025,8 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
 	    }
 	}
       else
-	warning (_("unable to open /proc file '%s'"), filename);
+	warning (_("unable to open /proc file '%ps'"),
+		 styled_string (file_name_style.style (), filename));
     }
   if (status_f)
     {
@@ -1031,7 +1036,8 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
       if (status)
 	gdb_puts (status.get ());
       else
-	warning (_("unable to open /proc file '%s'"), filename);
+	warning (_("unable to open /proc file '%ps'"),
+		 styled_string (file_name_style.style (), filename));
     }
   if (stat_f)
     {
@@ -1167,7 +1173,8 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
 #endif
 	}
       else
-	warning (_("unable to open /proc file '%s'"), filename);
+	warning (_("unable to open /proc file '%ps'"),
+		 styled_string (file_name_style.style (), filename));
     }
 }
 
@@ -2923,7 +2930,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 '%ps'"),
+	     styled_string (file_name_style.style (), filename));
 
   return false;
 }
-- 
2.55.0

[PATCH v3 4/8] gdbsupport: add extract_next_field helper

[Matthieu Longo]

Add extract_next_field, a helper for extracting a view from a buffer
up to a given separator. The helper also returns an iterator to the
beginning of the next entry, skipping consecutive separators.

The helper supports views whose value type is 'char', such as
gdb::array_view<char> and std::string_view (from c++20).
---
 gdb/unittests/common-utils-selftests.c | 32 +++++++++++++++++
 gdbsupport/common-utils.h              | 50 ++++++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/gdb/unittests/common-utils-selftests.c b/gdb/unittests/common-utils-selftests.c
index eb9c83616f0..bbd8b00a2de 100644
--- a/gdb/unittests/common-utils-selftests.c
+++ b/gdb/unittests/common-utils-selftests.c
@@ -19,6 +19,8 @@
 
 #include "gdbsupport/selftest.h"
 
+#include "gdbsupport/array-view.h"
+
 namespace selftests {
 
 /* Type of both 'string_printf' and the 'format' function below.  Used
@@ -125,6 +127,34 @@ string_vappendf_tests ()
   test_appendf_func (string_vappendf_wrapper);
 }
 
+static void
+extract_next_field_tests ()
+{
+  auto bytewise_match = [] (const auto &s1, const auto &s2) -> bool
+  {
+    return std::equal (s1.begin (), s1.end (), s2.begin (), s2.end ());
+  };
+
+  std::string s = "\n1234\nabc\n\ndef";
+  std::vector<std::string> expected = { "1234", "abc", "def" };
+  {
+    using view_t = gdb::array_view<char>;
+    std::vector<view_t> lines;
+    view_t buffer (s.data (), s.size ());
+    for (auto it = buffer.begin (); it != buffer.end ();)
+      {
+	auto [line, next_line_begin]
+	  = extract_next_field (buffer, it, '\n');
+	lines.emplace_back (line);
+	it = next_line_begin;
+      }
+
+    SELF_CHECK (expected.size () == lines.size ());
+    for (auto i = 0; i < expected.size () && i < lines.size (); ++i)
+      SELF_CHECK (bytewise_match (expected[i], lines[i]));
+  }
+}
+
 } /* namespace selftests */
 
 INIT_GDB_FILE (common_utils_selftests)
@@ -134,4 +164,6 @@ INIT_GDB_FILE (common_utils_selftests)
   selftests::register_test ("string_appendf", selftests::string_appendf_tests);
   selftests::register_test ("string_vappendf",
 			    selftests::string_vappendf_tests);
+  selftests::register_test ("extract_next_field",
+			    selftests::extract_next_field_tests);
 }
diff --git a/gdbsupport/common-utils.h b/gdbsupport/common-utils.h
index de83a715ac4..a4042b24644 100644
--- a/gdbsupport/common-utils.h
+++ b/gdbsupport/common-utils.h
@@ -279,4 +279,54 @@ struct string_view_hash
 
 } /* namespace gdb */
 
+/* Extract a view from BUFFER starting at START.
+
+   Leading occurrences of SEPARATOR at START are skipped.  The returned view
+   extends from the first non-separator character to the next occurrence of
+   SEPARATOR, or to BUFFER.end () if no separator is found.
+
+   Return the extracted view together with an iterator pointing to the first
+   non-separator character after the extracted view.  Successive separators
+   are skipped.  If there is no following non-separator character, the returned
+   iterator is BUFFER.end ().
+
+   If START does not point into BUFFER, return an empty view and BUFFER.end ().
+
+   ViewT may be gdb::array_view<char> or std::string_view.  The later requires
+   C++20, which added the constructor taking the range [first, last).  */
+
+template <class ViewT>
+std::pair<ViewT, typename ViewT::iterator>
+extract_next_field (ViewT buffer,
+		    typename ViewT::iterator start,
+		    char separator = '\0')
+{
+  static_assert (!std::is_reference_v <ViewT>);
+  static_assert (std::is_same_v <typename ViewT::value_type, char>);
+
+  auto next_start = buffer.end ();
+
+  /* Reject a START iterator that does not point into BUFFER.  */
+  if (start < buffer.begin () || start >= buffer.end ())
+    return {ViewT (), next_start};
+
+  /* Skip leading occurrences of the separator.  */
+  while (start != buffer.end () && *start == separator)
+    ++start;
+
+  auto it = std::find (start, buffer.end (), separator);
+
+  /* If no separator is found, the remainder of BUFFER is the final string.  */
+  if (it != buffer.end ())
+    {
+      /* Otherwise, skip successive separators so that NEXT_START points to
+	 the beginning of the next string, if any.  */
+      for (next_start = std::next (it);
+	   next_start != buffer.end () && *next_start == separator;
+	   next_start = std::next (next_start));
+    }
+
+  return {ViewT (start, it), next_start};
+}
+
 #endif /* GDBSUPPORT_COMMON_UTILS_H */
-- 
2.55.0

[PATCH v3 5/8] gdb/linux: refactor /proc mapping parsing to use array_view

[Matthieu Longo]

Refactor read_mapping and smaps parsing to operate on gdb::array_view<char>
instead of relying on NUL-terminated strings and strtok_r to split mapping
headers.

Track the amount of data returned by target_fileio_read_stralloc and use
extract_next_field to iterate over mapping lines.

Update read_mapping to parse fields using bounded iterators and diagnose
malformed addresses, separators, offsets, and inode values.

Adjust the callers of parse_smaps_data accordingly.
---
 gdb/linux-tdep.c | 167 +++++++++++++++++++++++++++++------------------
 1 file changed, 103 insertions(+), 64 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index bd4c1d5540f..772b561eb96 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -510,35 +510,60 @@ struct mapping
 /* Service function for corefiles and info proc.  */
 
 static mapping
-read_mapping (const char *line)
+read_mapping (gdb::array_view<char> line)
 {
   struct mapping mapping;
-  const char *p = line;
 
-  mapping.addr = strtoulst (p, &p, 16);
-  if (*p == '-')
-    p++;
-  mapping.endaddr = strtoulst (p, &p, 16);
+  decltype(line)::const_iterator it = line.begin ();
+  decltype(line)::const_iterator it_next;
 
-  p = skip_spaces (p);
-  const char *permissions_start = p;
-  while (*p && !c_isspace (*p))
-    p++;
-  mapping.permissions = std::string (permissions_start,
-				     (size_t) (p - permissions_start));
+  mapping.addr = strtoulst (it, &it_next, 16);
+  if (it == it_next)
+    error (_("failed to parse start address"));
+  else
+    it = it_next;
 
-  mapping.offset = strtoulst (p, &p, 16);
+  if (*it++ != '-')
+    error (_("expected separator '-'"));
 
-  p = skip_spaces (p);
-  const char *device_start = p;
-  while (*p && !c_isspace (*p))
-    p++;
-  mapping.device = {device_start, (size_t) (p - device_start)};
+  mapping.endaddr = strtoulst (it, &it_next, 16);
+  if (it == it_next)
+    error (_("failed to parse end address"));
+  else
+    it = it_next;
 
-  mapping.inode = strtoulst (p, &p, 10);
+  /* Skip spaces.  */
+  it = std::find_if_not (it, line.cend (), c_isspace);
 
-  p = skip_spaces (p);
-  mapping.filename = p;
+  it_next = std::find_if (it, line.cend (), c_isspace);
+  mapping.permissions = std::string (it, it_next);
+  it = it_next;
+
+  mapping.offset = strtoulst (it, &it_next, 16);
+  if (it == it_next)
+    error (_("failed to parse offset"));
+  else
+    it = it_next;
+
+  /* Skip spaces.  */
+  it = std::find_if_not (it, line.cend (), c_isspace);
+
+  it_next = std::find_if (it, line.cend (), c_isspace);
+  mapping.device = std::string (it, it_next);
+  it = it_next;
+
+  mapping.inode = strtoulst (it, &it_next, 10);
+  if (it == it_next)
+    error (_("failed to parse inode"));
+  else
+    it = it_next;
+
+  /* Skip spaces.  */
+  it = std::find_if_not (it, line.cend (), c_isspace);
+
+  mapping.filename = it;
+  /* Ensure that the line ends with '0'.  */
+  *line.end () = '\0';
 
   return mapping;
 }
@@ -985,8 +1010,9 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
   if (mappings_f)
     {
       xsnprintf (filename, sizeof filename, "/proc/%ld/maps", ptid.lwp ());
+      LONGEST len = 0;
       gdb::unique_xmalloc_ptr<char> map
-	= target_fileio_read_stralloc (NULL, filename);
+	= target_fileio_read_stralloc (NULL, filename, &len);
       if (map != NULL)
 	{
 	  gdb_printf (_("Mapped address spaces:\n\n"));
@@ -1001,12 +1027,13 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
 	  current_uiout->table_header (0, ui_left, "objfile", "File");
 	  current_uiout->table_body ();
 
-	  char *saveptr;
-	  for (const char *line = strtok_r (map.get (), "\n", &saveptr);
-	       line != nullptr;
-	       line = strtok_r (nullptr, "\n", &saveptr))
+	  gdb::array_view<char> content (map.get (), len);
+	  for (auto it = content.begin (); it != content.end ();)
 	    {
-	      struct mapping m = read_mapping (line);
+	      auto [line, next_line_begin]
+		= extract_next_field (content, it, '\n');
+	      it = next_line_begin;
+	      mapping m = read_mapping (line);
 
 	      ui_out_emit_tuple tuple_emitter (current_uiout);
 	      current_uiout->field_core_addr ("start", gdbarch, m.addr);
@@ -1559,20 +1586,22 @@ parse_smaps_key_value (const char *keyword, const char *line,
    into the SMAPS vector.  */
 
 static std::vector<smaps_data>
-parse_smaps_data (const char *data,
+parse_smaps_data (gdb::array_view<char> data,
 		  const std::string &maps_filename)
 {
-  char *line, *t;
-
-  gdb_assert (data != nullptr);
-
-  line = strtok_r ((char *) data, "\n", &t);
-
   std::vector<smaps_data> smaps;
 
-  while (line != NULL)
+  for (auto it = data.begin (); it != data.end ();)
     {
+      auto [region_header_line, next_line_begin]
+	= extract_next_field (data, it, '\n');
+      it = next_line_begin;
+
+      /* Parse a region's header.  */
+      mapping m = read_mapping (region_header_line);
+
       struct smaps_vmflags v;
+      memset (&v, 0, sizeof (v));
       int read, write, exec, priv;
       int has_anonymous = 0;
       int mapping_anon_p;
@@ -1580,8 +1609,6 @@ parse_smaps_data (const char *data,
       ULONGEST rss = -1;
       ULONGEST swap = -1;
 
-      memset (&v, 0, sizeof (v));
-      struct mapping m = read_mapping (line);
       mapping_anon_p = mapping_is_anonymous_p (m.filename);
       /* If the mapping is not anonymous, then we can consider it
 	 to be file-backed.  These two states (anonymous or
@@ -1614,7 +1641,8 @@ parse_smaps_data (const char *data,
 
       /* Try to detect if region should be dumped by parsing smaps
 	 counters.  */
-      for (line = strtok_r (NULL, "\n", &t);
+      char *line, *t;
+      for (line = strtok_r (it, "\n", &t);
 	   line != NULL && line[0] >= 'A' && line[0] <= 'Z';
 	   line = strtok_r (NULL, "\n", &t))
 	{
@@ -1679,26 +1707,31 @@ parse_smaps_data (const char *data,
 		}
 	    }
 	}
+
       /* Save the smaps entry to the vector.  */
-	smaps_data map;
-
-	map.start_address = m.addr;
-	map.end_address = m.endaddr;
-	map.filename = m.filename;
-	map.vmflags = v;
-	map.read = read? true : false;
-	map.write = write? true : false;
-	map.exec = exec? true : false;
-	map.priv = priv? true : false;
-	map.has_anonymous = has_anonymous;
-	map.mapping_anon_p = mapping_anon_p? true : false;
-	map.mapping_file_p = mapping_file_p? true : false;
-	map.offset = m.offset;
-	map.inode = m.inode;
-	map.rss = rss;
-	map.swap = swap;
-
-	smaps.emplace_back (map);
+      smaps_data map;
+
+      map.start_address = m.addr;
+      map.end_address = m.endaddr;
+      map.filename = m.filename;
+      map.vmflags = v;
+      map.read = read? true : false;
+      map.write = write? true : false;
+      map.exec = exec? true : false;
+      map.priv = priv? true : false;
+      map.has_anonymous = has_anonymous;
+      map.mapping_anon_p = mapping_anon_p? true : false;
+      map.mapping_file_p = mapping_file_p? true : false;
+      map.offset = m.offset;
+      map.inode = m.inode;
+      map.rss = rss;
+      map.swap = swap;
+
+      smaps.emplace_back (map);
+
+      if (line == nullptr)
+	break;
+      it = line;
     }
 
   return smaps;
@@ -1716,15 +1749,17 @@ linux_process_address_in_memtag_page (CORE_ADDR address)
   ptid_t ptid = get_ptid_for_slash_proc ();
   std::string smaps_file = string_printf ("/proc/%ld/smaps", ptid.lwp ());
 
+  LONGEST len = 0;
   gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (NULL, smaps_file.c_str ());
+    = target_fileio_read_stralloc (NULL, smaps_file.c_str (), &len);
 
   if (data == nullptr)
     return false;
 
   /* Parse the contents of smaps into a vector.  */
+  gdb::array_view<char> content (data.get (), len);
   std::vector<smaps_data> smaps
-    = parse_smaps_data (data.get (), smaps_file);
+    = parse_smaps_data (content, smaps_file);
 
   for (const smaps_data &map : smaps)
     {
@@ -1805,22 +1840,24 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
 
   std::string maps_filename = string_printf ("/proc/%ld/smaps", ptid.lwp ());
 
+  LONGEST len = 0;
   gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (NULL, maps_filename.c_str ());
+    = target_fileio_read_stralloc (NULL, maps_filename.c_str (), &len);
 
   if (data == NULL)
     {
       /* Older Linux kernels did not support /proc/PID/smaps.  */
       maps_filename = string_printf ("/proc/%ld/maps", ptid.lwp ());
-      data = target_fileio_read_stralloc (NULL, maps_filename.c_str ());
+      data = target_fileio_read_stralloc (NULL, maps_filename.c_str (), &len);
 
       if (data == nullptr)
 	return false;
     }
 
   /* Parse the contents of smaps into a vector.  */
+  gdb::array_view<char> content (data.get (), len);
   std::vector<smaps_data> smaps
-    = parse_smaps_data (data.get (), maps_filename);
+    = parse_smaps_data (content, maps_filename);
 
   for (const smaps_data &map : smaps)
     {
@@ -3263,14 +3300,16 @@ linux_address_in_shadow_stack_mem_range
   ptid_t ptid = get_ptid_for_slash_proc ();
   std::string smaps_file = string_printf ("/proc/%ld/smaps", ptid.lwp ());
 
+  LONGEST len = 0;
   gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (nullptr, smaps_file.c_str ());
+    = target_fileio_read_stralloc (nullptr, smaps_file.c_str (), &len);
 
   if (data == nullptr)
     return false;
 
+  gdb::array_view<char> content (data.get (), len);
   const std::vector<smaps_data> smaps
-    = parse_smaps_data (data.get (), smaps_file);
+    = parse_smaps_data (content, smaps_file);
 
   auto find_addr_mem_range = [&addr] (const smaps_data &map)
     {
-- 
2.55.0

[PATCH v3 6/8] gdb: introduce helper class target_file_reader

[Matthieu Longo]

Wrap all the boilerplate code required to read a file in a new helper
class: target_file_reader. 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, removes 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 target_file_reader. 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       | 121 ++++++++++++++++++-----------------------
 gdb/sparc64-tdep.c     |  17 +++---
 gdb/target.h           | 116 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 183 insertions(+), 83 deletions(-)

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 9b23db72bbe..f8d48084403 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1848,14 +1848,12 @@ 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)
+  target_file_reader<char> proc_status
+    (string_printf ("/proc/%d/status", inf->pid));
+  if (proc_status.empty_or_error ())
     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 +1865,7 @@ 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_path ()));
 
       return result;
     }
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 772b561eb96..0582e5eee64 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1737,6 +1737,12 @@ parse_smaps_data (gdb::array_view<char> data,
   return smaps;
 }
 
+static std::vector<smaps_data>
+parse_smaps_data (const target_file_reader<char> &freader)
+{
+  return parse_smaps_data (freader.view (), freader.path ());
+}
+
 /* Helper that checks if an address is in a memory tag page for a live
    process.  */
 
@@ -1747,19 +1753,14 @@ linux_process_address_in_memtag_page (CORE_ADDR address)
     return false;
 
   ptid_t ptid = get_ptid_for_slash_proc ();
-  std::string smaps_file = string_printf ("/proc/%ld/smaps", ptid.lwp ());
-
-  LONGEST len = 0;
-  gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (NULL, smaps_file.c_str (), &len);
 
-  if (data == nullptr)
+  target_file_reader<char> smaps_freader
+    (string_printf ("/proc/%ld/smaps", ptid.lwp ()));
+  if (smaps_freader.empty_or_error ())
     return false;
 
   /* Parse the contents of smaps into a vector.  */
-  gdb::array_view<char> content (data.get (), len);
-  std::vector<smaps_data> smaps
-    = parse_smaps_data (content, smaps_file);
+  std::vector<smaps_data> smaps = parse_smaps_data (smaps_freader);
 
   for (const smaps_data &map : smaps)
     {
@@ -1823,17 +1824,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)
+      target_file_reader<char> coredump_filter_freader
+	(string_printf ("/proc/%ld/coredump_filter", ptid.lwp ()));
+      if (!coredump_filter_freader.empty_or_error ())
 	{
 	  unsigned int flags;
 
-	  sscanf (coredumpfilterdata.get (), "%x", &flags);
+	  sscanf (coredump_filter_freader.data (), "%x", &flags);
 	  filterflags = (enum filter_flag) flags;
 	}
     }
@@ -1859,7 +1856,7 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
   std::vector<smaps_data> smaps
     = parse_smaps_data (content, maps_filename);
 
-  for (const 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))
@@ -2338,9 +2335,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.  */
@@ -2371,25 +2365,28 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
   const int leader_id = current_inferior ()->pid;
   const ptid_t live_ptid = get_ptid_for_slash_proc ();
 
-  /* Obtaining PID and filename.  */
-  xsnprintf (filename, sizeof (filename), "/proc/%ld/cmdline",
-	     live_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);
+  target_file_reader<gdb_byte> cmdline_freader
+    (string_printf ("/proc/%ld/cmdline", live_ptid.lwp ()));
+  if (cmdline_freader.empty_or_error ())
+    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);
+      warning (_("target file %ps does not contain a trailing null character"),
+	       styled_string (file_name_style.style (),
+			      cmdline_freader.c_path ()));
       return false;
     }
 
@@ -2399,27 +2396,26 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
   p->pr_pid = leader_id;
 
   /* 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", leader_id);
   /* 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')
+  const char *proc_stat = nullptr;
+  target_file_reader<char> stat_freader
+    (string_printf ("/proc/%d/stat", leader_id));
+  if (stat_freader.empty_or_error ()
+      || *(proc_stat = stat_freader.data ()) == '\0')
     {
       /* Despite being unable to read more information about the
 	 process, we return true here because at least we have its
@@ -2491,13 +2487,11 @@ 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", leader_id);
-  /* 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')
+  char *proc_status = nullptr;
+  target_file_reader<char> status_freader
+    (string_printf ("/proc/%d/status", leader_id));
+  if (status_freader.empty_or_error ()
+      || *(proc_status = status_freader.data ()) == '\0')
     {
       /* Returning true since we already have a bunch of information.  */
       return true;
@@ -2890,9 +2884,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;
 
@@ -2930,7 +2921,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
@@ -2940,15 +2931,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)
+  target_file_reader<char> task_maps_freader
+    (string_printf ("/proc/%ld/task/%ld/maps", pid, pid));
+  if (!task_maps_freader.empty_or_error ())
     {
       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))
 	{
@@ -2966,9 +2956,10 @@ linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
 	    }
 	}
     }
-  else
+  else if (task_maps_freader.error ())
     warning (_("unable to open /proc file '%ps'"),
-	     styled_string (file_name_style.style (), filename));
+	     styled_string (file_name_style.style (),
+			    task_maps_freader.c_path ()));
 
   return false;
 }
@@ -3298,18 +3289,12 @@ linux_address_in_shadow_stack_mem_range
     return false;
 
   ptid_t ptid = get_ptid_for_slash_proc ();
-  std::string smaps_file = string_printf ("/proc/%ld/smaps", ptid.lwp ());
-
-  LONGEST len = 0;
-  gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (nullptr, smaps_file.c_str (), &len);
-
-  if (data == nullptr)
+  target_file_reader<char> smaps_freader
+    (string_printf ("/proc/%ld/smaps", ptid.lwp ()));
+  if (smaps_freader.empty_or_error ())
     return false;
 
-  gdb::array_view<char> content (data.get (), len);
-  const std::vector<smaps_data> smaps
-    = parse_smaps_data (content, 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..5ed45809949 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -68,6 +68,7 @@
 #include <algorithm>
 #include "cli/cli-utils.h"
 #include "cli/cli-cmds.h"
+#include "cli/cli-style.h"
 #include "auxv.h"
 
 #define MAX_PROC_NAME_SIZE sizeof("/proc/99999/lwp/9999/adi/lstatus")
@@ -301,18 +302,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)
+  target_file_reader<char> adi_maps_freader
+    (string_printf ("/proc/%d/adi/maps", pid));
+  if (!adi_maps_freader.empty_or_error ())
     {
       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 +327,10 @@ 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 '%ps'"),
+	     styled_string (file_name_style.style (),
+			    adi_maps_freader.c_path ()));
 
   return false;
 }
diff --git a/gdb/target.h b/gdb/target.h
index 38bdc68f6a3..923e5c400b7 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2341,6 +2341,122 @@ 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 target_file_reader
+{
+  /* The path of the file being read.  */
+  std::string m_path;
+
+  /* Smart pointer to the data.  */
+  gdb::unique_xmalloc_ptr<T> m_data;
+
+  /* Number of bytes read.  */
+  LONGEST m_size;
+
+public:
+  /* Read the content of the file associated to PATH from the filesystem as
+     seen by INF.  If INF is NULL, use the filesystem seen by the debugger
+     (GDB or, for remote targets, the remote stub).  */
+  target_file_reader (const std::string &path, struct inferior *inf = nullptr)
+    : m_path (path)
+    , m_size (0)
+  {
+    /* The interface of target_fileio_read_stralloc and target_fileio_read_alloc
+       may appear inconsistent, but the difference is intentional.
+
+       On error, both functions return nullptr and set the size to a negative
+       value.  For a successful read of an empty file, however, the size is zero
+       and their return values differ:
+	 - target_fileio_read_stralloc returns an allocated empty string rather
+	   than nullptr.  The allocation contains the terminating '\0'.
+	 - target_fileio_read_alloc simply returns nullptr.
+
+       Hence, an assert in .data(), .view () and .cast_view () enforcing no
+       error but a valid buffer address.
+	 gdb_assert (!error () && m_data != nullptr);  */
+    if constexpr (std::is_same_v<T, char>)
+      m_data = target_fileio_read_stralloc (inf, m_path.c_str (), &m_size);
+    else
+      {
+	gdb_byte *buf = nullptr;
+	m_size = target_fileio_read_alloc (inf, m_path.c_str (), &buf);
+	m_data = gdb::unique_xmalloc_ptr<T> (reinterpret_cast<T *>(buf));
+      }
+  }
+
+  target_file_reader (target_file_reader &&) = default;
+  target_file_reader &operator= (target_file_reader &&) = default;
+
+  DISABLE_COPY_AND_ASSIGN (target_file_reader);
+
+  /* Return true if the file was read successfully but contained no data.  */
+  bool empty () const noexcept
+  { return m_size == 0; }
+
+  /* Return true if the file could not be read.  */
+  bool error () const noexcept
+  { return m_size < 0; }
+
+  /* Return true if an error occurred or the file was read successfully
+     but is empty.  */
+  bool empty_or_error () const noexcept
+  { return empty () || error (); }
+
+  /* Return a pointer to the data.  */
+  T *data () const noexcept
+  {
+    gdb_assert (!error () && m_data != nullptr);
+    return m_data.get ();
+  }
+
+  /* Return the number of bytes read.  */
+  LONGEST size () const noexcept
+  {
+    gdb_assert (!error ());
+    /* 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 view of the data.  */
+  gdb::array_view<T> view () const noexcept
+  {
+    gdb_assert (!error () && m_data != nullptr);
+    return gdb::array_view<T> (m_data.get (), size ());
+  }
+
+  /* Return a view of the data, reinterpreted as objects of type U.
+     The size of the underlying storage must be an exact multiple of sizeof (U),
+     and the storage must be suitably aligned for U.  */
+  template <typename U>
+  gdb::array_view<U> cast_view () const noexcept
+  {
+    gdb_assert (!error () && m_data != nullptr);
+
+    size_t nbytes = size () * sizeof (T);
+
+    /* The number of bytes must be a multiple of sizeof(U).
+       Do not silently discard trailing bytes.  */
+    gdb_assert (nbytes % sizeof (U) == 0);
+
+    /* The underlying storage must satisfy U's alignment requirement.  */
+    gdb_assert (reinterpret_cast<uintptr_t> (m_data.get ()) % alignof (U) == 0);
+
+    return gdb::array_view<U> (reinterpret_cast<U *> (m_data.get ()),
+			       nbytes / sizeof (U));
+  }
+
+  /* Return the path of the file that was read.  */
+  const std::string &path () const noexcept
+  { return m_path; }
+
+  /* Return the path of the file that was read as a C string.  */
+  const char *c_path () const noexcept
+  { return m_path.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

[PATCH v3 7/8] gdb/linux-tdep: use target_file_reader for procfs parsing

[Matthieu Longo]

Migrate procfs reads in linux_info_proc and linux_find_memory_regions_full
from the target_fileio_* allocation helpers to target_file_reader.

Use the array views provided by target_file_reader to simplify the parsing
of procfs contents. In particular, use gdb::ranges::replace for the
NUL-separated command line and extract_view_from_buffer for environment
entries.

Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
---
 gdb/linux-tdep.c | 120 ++++++++++++++++++++++-------------------------
 1 file changed, 57 insertions(+), 63 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 0582e5eee64..bb1870efe08 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -45,6 +45,7 @@
 #include "memtag.h"
 #include "cli/cli-style.h"
 #include "gdbsupport/unordered_map.h"
+#include "gdbsupport/ranges.h"
 
 #include <algorithm>
 
@@ -939,26 +940,26 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
 
   if (cmdline_f)
     {
-      xsnprintf (filename, sizeof filename, "/proc/%ld/cmdline", ptid.lwp ());
-      gdb_byte *buffer;
-      LONGEST len = target_fileio_read_alloc (nullptr, filename, &buffer);
-
-      if (len > 0)
+      std::string path = string_printf ("/proc/%ld/cmdline", ptid.lwp ());
+      target_file_reader<gdb_byte> cmdline_freader (path);
+      if (!cmdline_freader.empty_or_error ())
 	{
-	  gdb::unique_xmalloc_ptr<char> cmdline ((char *) buffer);
-	  ssize_t pos;
-
-	  for (pos = 0; pos < len - 1; pos++)
-	    {
-	      if (buffer[pos] == '\0')
-		buffer[pos] = ' ';
-	    }
-	  buffer[len - 1] = '\0';
-	  gdb_printf ("cmdline = '%s'\n", buffer);
+	  /* /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> ();
+	  if (cmdline[cmdline.size () - 1] != '\0')
+	    warning (_("malformed '%ps', missing null-terminating character"),
+		     styled_string (file_name_style.style (), path.c_str ()));
+	  /* Replace null characters splitting the arguments in the command
+	     line by spaces, except for the last one.  */
+	  gdb::ranges::replace
+	    (cmdline.slice (0, cmdline.size () - 1), '\0', ' ');
+	  gdb_printf ("cmdline = '%s'\n", cmdline.data ());
 	}
       else
 	warning (_("unable to open /proc file '%ps'"),
-		 styled_string (file_name_style.style (), filename));
+		 styled_string (file_name_style.style (),
+				cmdline_freader.c_path ()));
     }
   if (cwd_f)
     {
@@ -973,28 +974,26 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
     }
   if (environ_f)
     {
-      xsnprintf (filename, sizeof filename, "/proc/%ld/environ", ptid.lwp ());
-      gdb_byte *buffer;
-      LONGEST len = target_fileio_read_alloc (nullptr, filename, &buffer);
-
-      if (len > 0)
+      target_file_reader<gdb_byte> environ_freader
+	(string_printf ("/proc/%ld/environ", ptid.lwp ()));
+      if (!environ_freader.empty_or_error ())
 	{
-	  gdb::unique_xmalloc_ptr<char> dealloc ((char *) buffer);
 	  gdb_printf (_("Environment variables:\n\n"));
-
+	  gdb::array_view<char> buffer = environ_freader.cast_view<char> ();
 	  /* Entries are separated by the null character.
 	     Print each environment variable, line by line.  */
-	  gdb_byte *buffer_end = buffer + len;
-	  while (buffer < buffer_end)
+	  for (auto it = buffer.begin (); it != buffer.end ();)
 	    {
-	      gdb_printf ("  %s\n", buffer);
-	      /* +1 for the null character.  */
-	      buffer += strlen ((char *) buffer) + 1;
+	      auto [ntbs, next_start]
+		= extract_next_field (buffer, it, '\0');
+	      gdb_printf ("  %s\n", ntbs.data ());
+	      it = next_start;
 	    }
 	}
       else
 	warning (_("unable to open /proc file '%ps'"),
-		 styled_string (file_name_style.style (), filename));
+		 styled_string (file_name_style.style (),
+				environ_freader.c_path ()));
     }
   if (exe_f)
     {
@@ -1009,11 +1008,9 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
     }
   if (mappings_f)
     {
-      xsnprintf (filename, sizeof filename, "/proc/%ld/maps", ptid.lwp ());
-      LONGEST len = 0;
-      gdb::unique_xmalloc_ptr<char> map
-	= target_fileio_read_stralloc (NULL, filename, &len);
-      if (map != NULL)
+      target_file_reader<char> map_freader
+	(string_printf ("/proc/%ld/maps", ptid.lwp ()));
+      if (!map_freader.empty_or_error ())
 	{
 	  gdb_printf (_("Mapped address spaces:\n\n"));
 	  ui_out_emit_table emitter (current_uiout, 6, -1, "ProcMappings");
@@ -1027,12 +1024,13 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
 	  current_uiout->table_header (0, ui_left, "objfile", "File");
 	  current_uiout->table_body ();
 
-	  gdb::array_view<char> content (map.get (), len);
+	  auto content = map_freader.view ();
 	  for (auto it = content.begin (); it != content.end ();)
 	    {
 	      auto [line, next_line_begin]
 		= extract_next_field (content, it, '\n');
 	      it = next_line_begin;
+
 	      mapping m = read_mapping (line);
 
 	      ui_out_emit_tuple tuple_emitter (current_uiout);
@@ -1053,27 +1051,27 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
 	}
       else
 	warning (_("unable to open /proc file '%ps'"),
-		 styled_string (file_name_style.style (), filename));
+		 styled_string (file_name_style.style (),
+				map_freader.c_path ()));
     }
   if (status_f)
     {
-      xsnprintf (filename, sizeof filename, "/proc/%ld/status", ptid.lwp ());
-      gdb::unique_xmalloc_ptr<char> status
-	= target_fileio_read_stralloc (NULL, filename);
-      if (status)
-	gdb_puts (status.get ());
+      target_file_reader<char> status_freader
+	(string_printf ("/proc/%ld/status", ptid.lwp ()));
+      if (!status_freader.empty_or_error ())
+	gdb_puts (status_freader.data ());
       else
 	warning (_("unable to open /proc file '%ps'"),
-		 styled_string (file_name_style.style (), filename));
+		 styled_string (file_name_style.style (),
+				status_freader.c_path ()));
     }
   if (stat_f)
     {
-      xsnprintf (filename, sizeof filename, "/proc/%ld/stat", ptid.lwp ());
-      gdb::unique_xmalloc_ptr<char> statstr
-	= target_fileio_read_stralloc (NULL, filename);
-      if (statstr)
+      target_file_reader<char> stat_freader
+	(string_printf ("/proc/%ld/stat", ptid.lwp ()));
+      if (!stat_freader.empty_or_error ())
 	{
-	  const char *p = statstr.get ();
+	  const char *p = stat_freader.data ();
 
 	  gdb_printf (_("Process: %s\n"),
 		      pulongest (strtoulst (p, &p, 10)));
@@ -1201,7 +1199,8 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
 	}
       else
 	warning (_("unable to open /proc file '%ps'"),
-		 styled_string (file_name_style.style (), filename));
+		 styled_string (file_name_style.style (),
+				stat_freader.c_path ()));
     }
 }
 
@@ -1835,27 +1834,22 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
 	}
     }
 
-  std::string maps_filename = string_printf ("/proc/%ld/smaps", ptid.lwp ());
-
-  LONGEST len = 0;
-  gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (NULL, maps_filename.c_str (), &len);
+  std::vector<smaps_data> smaps;
 
-  if (data == NULL)
+  target_file_reader<char> smaps_freader
+    (string_printf ("/proc/%ld/smaps", ptid.lwp ()));
+  if (!smaps_freader.empty_or_error ())
+    smaps = parse_smaps_data (smaps_freader);
+  else
     {
       /* Older Linux kernels did not support /proc/PID/smaps.  */
-      maps_filename = string_printf ("/proc/%ld/maps", ptid.lwp ());
-      data = target_fileio_read_stralloc (NULL, maps_filename.c_str (), &len);
-
-      if (data == nullptr)
+      target_file_reader<char> maps_freader
+	(string_printf ("/proc/%ld/maps", ptid.lwp ()));
+      if (maps_freader.empty_or_error ())
 	return false;
+      smaps = parse_smaps_data (maps_freader);
     }
 
-  /* Parse the contents of smaps into a vector.  */
-  gdb::array_view<char> content (data.get (), len);
-  std::vector<smaps_data> smaps
-    = parse_smaps_data (content, maps_filename);
-
   for (const smaps_data &map: smaps)
     {
       /* Invoke the callback function to create the corefile segment.  */
-- 
2.55.0

[PATCH v3 8/8] gdb/linux-tdep: remove legacy parse_smaps_data overload

[Matthieu Longo]

Now that all callers use the target_file_reader overload of parse_smaps_data,
the legacy interface taking a raw buffer and file path separately is no
longer needed.

Fold its implementation into the target_file_reader version and remove the
obsolete wrapper.  This also simplifies the implementation by using the
target_file_reader accessors directly.

Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Reviewed-By: Luis Machado <luis.machado.foss@gmail.com>
---
 gdb/linux-tdep.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index bb1870efe08..c928338406a 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1581,14 +1581,14 @@ parse_smaps_key_value (const char *keyword, const char *line,
 /* Helper function to parse the contents of /proc/<pid>/smaps into a data
    structure, for easy access.
 
-   DATA is the contents of the smaps file.  The parsed contents are stored
-   into the SMAPS vector.  */
+   FREADER is a wrapper around the contents of the smaps file.
+   The parsed contents are returned as a vector.  */
 
 static std::vector<smaps_data>
-parse_smaps_data (gdb::array_view<char> data,
-		  const std::string &maps_filename)
+parse_smaps_data (const target_file_reader<char> &freader)
 {
   std::vector<smaps_data> smaps;
+  auto data = freader.view ();
 
   for (auto it = data.begin (); it != data.end ();)
     {
@@ -1649,8 +1649,9 @@ parse_smaps_data (gdb::array_view<char> data,
 
 	  if (sscanf (line, "%64s", keyword) != 1)
 	    {
-	      warning (_("Error parsing {s,}maps file '%s'"),
-		       maps_filename.c_str ());
+	      warning (_("error parsing keyword in {s,}maps file '%ps'"),
+		       styled_string (file_name_style.style (),
+				      freader.c_path ()));
 	      break;
 	    }
 
@@ -1664,12 +1665,12 @@ parse_smaps_data (gdb::array_view<char> data,
 	    decode_vmflags (line, &v);
 
 	  if (parse_smaps_key_value (keyword, line, "Rss:",
-				     maps_filename,
+				     freader.path (),
 				     &rss))
 	    continue;
 
 	  if (parse_smaps_key_value (keyword, line, "Swap:",
-				     maps_filename,
+				     freader.path (),
 				     &swap))
 	    continue;
 
@@ -1680,8 +1681,11 @@ parse_smaps_data (gdb::array_view<char> data,
 
 	      if (sscanf (line, "%*s%lu", &number) != 1)
 		{
-		  warning (_("Error parsing {s,}maps file '%s' number"),
-			   maps_filename.c_str ());
+		  warning (_("error parsing numeric value associated with "
+			     "key '%s' in {s,}maps file '%ps'"),
+			   keyword,
+			   styled_string (file_name_style.style (),
+					  freader.c_path ()));
 		  break;
 		}
 	      if (number > 0)
@@ -1736,12 +1740,6 @@ parse_smaps_data (gdb::array_view<char> data,
   return smaps;
 }
 
-static std::vector<smaps_data>
-parse_smaps_data (const target_file_reader<char> &freader)
-{
-  return parse_smaps_data (freader.view (), freader.path ());
-}
-
 /* Helper that checks if an address is in a memory tag page for a live
    process.  */
 
-- 
2.55.0