Re: patch 0/2 debuginfod submission

[Mark Wielaard <mark@klomp.org>]

Hi Frank, Hi Aaron,

On Mon, 2019-10-28 at 15:04 -0400, Frank Ch. Eigler wrote:
> Aaron Merey and I would like to propose debuginfod for merge into
> elfutils mainline, after a couple of months of work.  As a reminder,
> this is an http server (written in C++11) for debuginfo-related
> artifacts (elf/dwarf/source files), plus a C client library & command
> line tool.  It comes with man pages and tests.  
> 
> I'll followup with two git format-patch emails, one for the client,
> and one for the server+tests+etc.  The identical code is also in the
> git://sourceware.org/git/elfutils.git debuginfod-submit branch.

Thanks, very interesting. For those that don't know, there was also a
GNU Tools Cauldron presentation about this work:
https://gcc.gnu.org/wiki/cauldron2019#cauldron2019talks.The_elfutils_debuginfo_server

I only browsed through the code quickly, but I like what I see.
For now just a comment on the libdwfl integration.

It is guarded by ENABLE_DEBUGINFOD, which is off by default.
I think the support should always be enabled in libdwfl whether or not
the debuginfod server is build or not, or that it should be guarded by
something like ENABLE_DEBUGINFOD_CLIENT_SUPPORT (which would default to
on by default). It already "works" whether or not the debuginfod
library and/or server are there. So lets default to enable support in
libdwfl, so people can easily build elfutils/libdw separately from
elfutils/debuginfod and combine the result into something that just
works.

Also I think you should cache a negative result for
fp_debuginfod_find_debuginfo (say assign it (void *) -1) so you don't
keep trying to find the library/symbol each and every time.

Having parallel code on my mind I am worried now how this works when
called concurrently from two threads. There is a lot of code in libdwfl
that isn't concurrent-safe at the moment. But if possible lets not
introduce more. Not a big concern, but nice if you could give it a
thought.

Similarly, our error reporting is already pretty poor, so you aren't
making things worse. But have you thought about a way for the libdwfl
user to provide some way to indicate why something couldn't be
resolved/found? Again, not really a big concern since the current code
already has very limited/poor error reporting, but maybe you have
thoughts about it?

Have you thought about just calling debuginfod-find from the libdwfl
code? Or is execing from a library really just a no-no?

Thanks,

Mark