Re: [PATCH 1/3] gdbserver: make arch and osabi names gdb::unique_xmalloc_ptr<char>

[Andrew Burgess <aburgess@redhat.com>]

Simon Marchi <simark@simark.ca> writes:

> On 2024-10-09 07:07, Andrew Burgess wrote:
>> Simon Marchi <simark@simark.ca> writes:
>> 
>>> On 2024-10-06 14:37, Andrew Burgess wrote:
>>>> Convert target_desc::arch and target_desc::osabi from 'const char*' to
>>>> gdb::unique_xmalloc_ptr<char>.  This also allows us to remove the user
>>>> defined ~target_desc destructor.
>>>>
>>>> I doubt it ever actually occurred, but in theory at least, there was a
>>>> memory leak in set_tdesc_architecture and set_tdesc_osabi where the
>>>> member variables were assigned without freeing any previous
>>>> value... but I suspect that usually these fields are only set once.
>>>>
>>>> There should be no user visible changes after this commit.
>>>
>>> Could they be std::string instead?
>> 
>> I considered this, but that would, I think, require wider reaching
>> changes as the API to add the osabi to the returned tdesc passes the
>> information via 'const char *'.  This approach added the memory safety
>> without requiring any further changes.
>> 
>> If you'd prefer std::string be used then let me know and I'll update
>> things.
>
> Up to you, I don't mind, it can be done later.

I'll leave it then if that's OK.

My reason is that, if the osbi member variable changes to std::string
then it makes sense that tdesc_osabi_name return 'const std::string &'.
Which means that tdesc_osabi_name in GDB will also need to return 'const
std::string &', but on the GDB side we store 'enum gdb_osabi' rather
than the actual string ... so now that code needs updating too.

I only wanted to fix this so that I could call set_tdesc_osbi without
leaking memory, and gdb::unique_xmalloc_ptr<char>, which we use in loads
of places, achieves that, and leaves the API untouched.

But I'm not against changing to std::string if someone's keen :)

Thanks,
Andrew