Re: [PATCH 4/11] gdb/solib-rocm: add cached_fd to manage cached fd lifetime

[Simon Marchi <simon.marchi@efficios.com>]

On 5/7/26 4:17 PM, Lancelot SIX wrote:
> On Tue, Dec 09, 2025 at 02:32:08PM -0500, Simon Marchi wrote:
>> Make the management of the cached fds automatic.
>>
>> Change rocm_solib_fd_cache::open to return a cached_fd, an RAII type
>> that calls rocm_solib_fd_cache::close on destruction.  cached_fd holds
>> the actual fd and a reference to the cache it comes from.
>>
>> Change rocm_code_object_stream_file to hold a cached_fd and remove the
>> explicit destructor.
>>
>> This is not essential, I just thought it would be nice.
>>
>> Change-Id: I2529ac1caebbe6b7e6e60d228a403064f6f38d06
>> ---
>>  gdb/solib-rocm.c | 91 ++++++++++++++++++++++++++++++------------------
>>  1 file changed, 58 insertions(+), 33 deletions(-)
>>
>> diff --git a/gdb/solib-rocm.c b/gdb/solib-rocm.c
>> index bde34bafd8b1..5560a6035184 100644
>> --- a/gdb/solib-rocm.c
>> +++ b/gdb/solib-rocm.c
>> @@ -39,22 +39,58 @@ struct rocm_solib_fd_cache
>>    explicit rocm_solib_fd_cache (inferior *inf) : m_inferior (inf) {}
>>    DISABLE_COPY_AND_ASSIGN (rocm_solib_fd_cache);
>>  
>> +  /* The open method returns an object of this type.
>> +
>> +     On destruction, automatically call close to indicate the caller no longer
>> +     uses the fd.  */
>> +  struct cached_fd
>> +  {
>> +    cached_fd (int fd, rocm_solib_fd_cache &cache)
>> +      : fd (fd),
>> +	cache (cache)
>> +    {
>> +    }
>> +
>> +    cached_fd (cached_fd &&other)
>> +      : fd (other.fd),
>> +	cache (other.cache)
>> +    {
>> +      other.fd = -1;
>> +    }
>> +
>> +    ~cached_fd ()
>> +    {
>> +      if (fd == -1)
>> +	return;
>> +
>> +      fileio_error target_errno;
>> +      if (cache.close (fd, &target_errno) != 0)
>> +	warning (_("Failed to close solib: %s"),
>> +		 strerror (fileio_error_to_host (target_errno)));
>> +    }
>> +
>> +    DISABLE_COPY_AND_ASSIGN (cached_fd);
>> +
>> +    int fd;
> 
> Given some changes since this patch was sent, this is now a target_fd.

Ok, I will rename the type to cached_target_fd.

> 
>> +    rocm_solib_fd_cache &cache;
> 
> the cache field is never accessed outside of the class, there is no need
> to have it public.

Ok, while at it I made the fd field private too and added a getter.

> I expect quite a few conflicts when rebasing this patch, nevertheless
> the direction so far works for me.

Not too bad, because the fd didn't change meaning, it juste changed to a
stricter type.

Simon