Re: patch 0/2 debuginfod submission

["Frank Ch. Eigler" <fche at redhat dot com>]

Hi, Mark -


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

Righto.


> 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). [...]

OK, sure, IMO don't even bother with a guard.  It's just a dlopen/dlsym,
which is portable.  Will update the first patch on the branch with that.


> 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.

Will look into that later on.  With the debuginfod server, a negative
hit comes back instantly (no file searching, just an indexed database
lookup), so this may not be a big deal.


> 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.

The new client-side code doesn't call into elfutils though, so does
not amplify hypothetical problems there.  In fact I'm not sure it's
multithreaded at all, or just nonblocking I/O.

The server is multithreaded, but that's relatively unavoidable, and
turns out to work without any valgrind complaints (last time I tried).


> 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?

We document returning standard errnos generally, and a few particular
ones for network unreachability.

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

It's not great as a practice.  (We do that from debuginfod to popen
rpm2cpio but reluctantly, and carefully with signals.)  Another reason
for calling directly is avoidance of a race condition, wherein two
debuginfod-client processes run at the same time, and one cleans the
cache right under the foot of the other.  From the C API, you're
guaranteed to get a usable fd, even if the underlying file was
unlinked while you were looking.  An fd is all elfutils needs (which I
love).


- FChE