Re: [PATCH] debuginfod-support.c: Use long-lived debuginfod_client

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-05-06 2:47 p.m., Frank Ch. Eigler wrote:> Hi -
> 
>> I think it would still be a good idea to properly close the client
>> at exit.  Not because the memory leak is a concern, but because we
>> shouldn't assume what closing a debuginfod client (current or
>> future) does or does not do, so we shouldn't assume that we can get
>> away without calling debuginfo_end.
> 
> If it's not hard to do, sure .... but wouldn't that necessitate
> exposing the pointer from function static scope again?

I don't think so, just make the static variable a unique pointer (the
type that you are removing in this patch).  The destructor of the
unique_ptr will be called at program exit, closing the client cleanly.
That variable can be declared inside debuginfod_init, like Tom
suggested.  Something like:

static debuginfo_client *
debuginfod_init ()
{
  static debuginfod_client_up global_client;

  if (global_client == nullptr)
    global_client.reset (debuginfod_begin ());

  return global_client.get ();
}

In that case, the debuginfod_init function should probably be renamed to
something like get_debuginfod_client, something like that.

>> A debuginfod client could eventually maintain some temporary files
>> that need to be deleted when closing in order not to litter /tmp,
>> some cache files need to be flushed, etc.
> 
> I suspect that if we were to do any of those things in the future,
> we'd keep things clean by techniques like holding onto fd's to
> unlinked files, so process exit is enough.  (We could make a
> commitment in the docs.)

That sounds like a good idea, this way even if the user of the client
crashes, there won't be any leftovers.  However, committing to that in
the documentation just sounds like an unnecessary way to paint yourself
in a corner.

Simon