Re: [PATCH v1 08/10] gdb/linux-tdep: migrate parse_smaps_data to file_reader_t

Luis <[email protected]> Tue, 21 Jul 2026 22:49:34 +0100
Newsgroups gmane.comp.gdb.patches
Message-ID <[email protected]>
Empty commit message.

On 07/07/2026 16:48, Matthieu Longo wrote:
> ---
>   gdb/linux-tdep.c | 30 ++++++++++++------------------
>   1 file changed, 12 insertions(+), 18 deletions(-)
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index f1cea3040f9..dbc69a5a7fc 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -1530,18 +1530,17 @@ 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 stored into the SMAPS vector.  */
>   
>   static std::vector<struct smaps_data>
> -parse_smaps_data (const char *data,
> -		  const std::string &maps_filename)
> +parse_smaps_data (const file_reader_t<char> &freader)
>   {
>     char *line, *t;
>   
> -  gdb_assert (data != nullptr);
> +  gdb_assert (freader);
>   
> -  line = strtok_r ((char *) data, "\n", &t);
> +  line = strtok_r (freader.data (), "\n", &t);
>   
>     std::vector<struct smaps_data> smaps;
>   
> @@ -1597,8 +1596,8 @@ parse_smaps_data (const 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 '%s'"),
> +		       freader.c_filepath ());
>   	      break;
>   	    }
>   
> @@ -1612,12 +1611,12 @@ parse_smaps_data (const char *data,
>   	    decode_vmflags (line, &v);
>   
>   	  if (parse_smaps_key_value (keyword, line, "Rss:",
> -				     maps_filename,
> +				     freader.filepath (),
>   				     &rss))
>   	    continue;
>   
>   	  if (parse_smaps_key_value (keyword, line, "Swap:",
> -				     maps_filename,
> +				     freader.filepath (),
>   				     &swap))
>   	    continue;
>   
> @@ -1628,8 +1627,9 @@ parse_smaps_data (const 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 '%s'"),
> +			   keyword, freader.c_filepath ());
>   		  break;
>   		}
>   	      if (number > 0)
> @@ -1679,12 +1679,6 @@ 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.  */
>   

Otherwise looks reasonable.

Reviewed-By: Luis Machado <[email protected]>