On 8/22/23 16:43, Pedro Alves wrote:
> On 2023-08-21 11:53, Tom de Vries via Gdb-patches wrote:
>> In remote_target::thread_info_to_thread_handle we return a copy:
>> ...
>> gdb::byte_vector
>> remote_target::thread_info_to_thread_handle (struct thread_info *tp)
>> {
>>    remote_thread_info *priv = get_remote_thread_info (tp);
>>    return priv->thread_handle;
>> }
>> ...
>>
>> Fix this by returning a const reference instead:
>> ...
>> const gdb::optional<gdb::byte_vector> &
>> remote_target::thread_info_to_thread_handle (struct thread_info *tp)
>> ...
>>
>> Returning a gdb::optional allows us to return a nullptr, or std::nullopt in
>> std::optional terms, something that is required by
>> thread_db_target::thread_info_to_thread_handle.
>>
>> In gdb we use gdb::optional instead std::optional, because std::optional is
>> availabe starting c++17 and we support c++11 and c++14, but gdb::nullopt is
>> currently not available, though a submission is available [1].
>>
>> So we use a kludge gdb_optional_byte_vector_nullopt.
>>
> 
> IMHO making the function return gdb::array_view<const gdb_byte>
> would be even better.  Then the byte_vector is completely an implementation
> detail, and, you wouldn't need to wrap with optional, as you could just
> return an empty array_view, like currently we return an empty byte_vector.
> 

Hi Pedro,

thanks for the review.

I've given the gdb::array_view<const gdb_byte> a try, and that works 
fine, and greatly simplifies the patch, so thanks for that suggestion.

The first patch is no longer necessary, but we could consider committing 
it nevertheless, since the work is done and may be needed in the future.

Thanks,
- Tom