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