Re: [PATCH v1 06/10] gdb/linux-tdep: migrate linux_info_proc to file_reader_t

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

On 09/07/2026 07:35, Thiago Jung Bauermann wrote:
> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> The patch migratse the code of linux_info_proc to use file_reader_t to
> 
> Typo: migrates
> 

Fixed.

>> read the procfs files.
>> The availability of array_views allows to also simplify the logic in
>> several places, where null-terminating characters are replaced by spaces,
>> or where the file content is iterated line by line.
>> In the last case, a new helper function, extract_string_view_from_buffer,
>> encapsulates the logic for such iterations where string are separated by
>> tokens.
>> ---
>>  gdb/linux-tdep.c | 120 +++++++++++++++++++++++++++--------------------
>>  1 file changed, 69 insertions(+), 51 deletions(-)
> 
> Nice improvement, thanks!
> 
> Just a couple of nits, but:
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> 
>> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
>> index a12a69f03a2..a2af1586d35 100644
>> --- a/gdb/linux-tdep.c
>> +++ b/gdb/linux-tdep.c
>> @@ -836,6 +836,27 @@ dump_note_entry_p (filter_flags filterflags, const smaps_data &map)
>>    return true;
>>  }
>>  
>> +/* In a character buffer where entries are separated by a SEPARATOR character,
>> +   extract the string view starting at START.
>> +   Return the extracted view and the iterator to the next entry.  */
>> +
>> +static std::pair<gdb::array_view<char>, gdb::array_view<char>::iterator>
>> +extract_string_view_from_buffer (gdb::array_view<char> &buffer,
>> +				 gdb::array_view<char>::iterator start,
>> +				 char separator = '\0')
>> +{
>> +  if (start < buffer.begin () || start >= buffer.end ())
>> +    return std::make_pair (gdb::array_view<char> (), buffer.end ());
>> +
>> +  auto it = std::find (start, buffer.end (), separator);
>> +  if (it == buffer.end ())
>> +    return std::make_pair (gdb::array_view<char> (), buffer.end ());
>> +
>> +  auto next_start = std::next (it);
>> +  return std::make_pair
>> +    (gdb::array_view<char> (start, next_start), next_start);
> 
> This line fits 80 columns and doesn't need to be broken.
> 

Fixed.

>> +}
>> +
>>  /* Implement the "info proc" command.  */
>>  
>>  static void
>> @@ -878,25 +899,20 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
>>    gdb_printf (_("process %d\n"), ptid.pid ());
>>    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)
>> +      file_reader_t<gdb_byte> cmdline_freader
>> +	(string_printf ("/proc/%ld/cmdline", ptid.lwp ()));
>> +      if (cmdline_freader)
>>  	{
>> -	  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);
>> +	  gdb::array_view<char> cmdline = cmdline_freader.cast_view<char> ();
>> +	  gdb_assert (cmdline[ cmdline.size () - 1] == '\0');
> 
> Extraneous space after '['.
> 

Fixed.

>> +	  /* Replace null characters splitting the arguments in the command
>> +	     line by spaces, except for the last one.  */
>> +	  gdb::replace (cmdline.slice (0, cmdline.size () - 1), '\0', ' ');
>> +	  gdb_printf ("cmdline = '%s'\n", cmdline.data ());
>>  	}
>>        else
>> -	warning (_("unable to open /proc file '%s'"), filename);
>> +	warning (_("unable to open /proc file '%s'"),
>> +		 cmdline_freader.c_filepath());
>>      }
>>    if (cwd_f)
>>      {
> 

Matthieu