Re: [PATCH] gdb: make inferior::terminal a unique ptr

[Christian Biesinger <cbiesinger@google.com>]

On Thu, Jun 25, 2020 at 1:09 PM Pedro Alves <pedro@palves.net> wrote:
>
> On 6/25/20 6:59 PM, Luis Machado wrote:
> > On 6/25/20 2:28 PM, Pedro Alves wrote:
> >> On 6/25/20 6:18 PM, Christian Biesinger via Gdb-patches wrote:
> >>> On Thu, Jun 25, 2020 at 10:55 AM Simon Marchi via Gdb-patches
> >>> <gdb-patches@sourceware.org> wrote:
> >>>> -    current_inferior ()->terminal = xstrdup (terminal_name);
> >>>> +    current_inferior ()->terminal.reset (xstrdup (terminal_name));
> >>>
> >>> Perhaps it really should be a std::string?
> >>
> >> I think there's no good reason for that.  It's a string that doesn't
> >> ever need to grow/shrink.  And then its use is to pass down to
> >> a syscall that accepts C-style null-terminated strings:
> >
> > I tend to agree with Christian. I suppose one positive outcome is to make the code less cumbersome. It's not like xstrdup is great to read anyway.
> >
> > Are we worried about performance implications of passing std::string's around like this?
>
> It's more like -- why store a fat 32 bytes object in the inferior when
> a pointer does just as well.  There's no code that would benefit from
> making this a string, like, there's no strlen call that that would avoid.
> The code that wants to consume this wants a pointer.  A method-based
> interface like I pointed at in that github commit exposes a pointer.
> So it would just be implementation detail.

Well, if the string is usually short, std::string can avoid a memory
allocation...

But I guess it comes down to a philosophical question on whether
xstrdup/unique_xmalloc_ptr or std::string is more elegant.

Christian