Re: [PATCH 0/4] Improve elfutils diagnostics

[Jonathan Lebon <jlebon at redhat dot com>]

Hello Mark,

> > The first two patches enable elfutils to give a more specific error when
> > compressed sections fail to be decompressed.
> 
> These look good and useful.
> I added ChangeLog entries and pushed them to master.

Thanks!

> I am still pondering these two patches. The idea is sane. Some
> operations are compound and a simple error code/string only tells you
> something failed, but not really why or what was done. There are two or
> three dwfl functions that would benefit from being able to report more
> clearly what went wrong. dwfl_module_getdwarf () which you address in
> patch 4. But also dwfl_module_getelf () and maybe dwfl_module_getsymtab
> (). Which all do some try-fail-try-something-else lookups.

Right, I only added support for getdwarf(). Once we settle on an interface,
I might have a look at the other ones if I have enough time.

> getdwarf and
> getelf both allow the user to plug in their own lookup callbacks to make
> things things even more interesting.

Yes, these patches only report what the default callbacks do. I guess it
might be good to provide a way for callbacks to write their errors to the
same spot, although I suppose user-provided callbacks also have their own
way of reporting to the user things that went wrong.

> One implementation detail that I like to see changed is to make the
> details errmsg a list of strings instead of one big string (and
> similarly for dw_tried_paths). That makes things a bit cheaper when the
> result isn't actually used. You don't have to concatenate the strings
> beforehand and the user can decide how to use the separate detail
> messages.

E.g. a linked list of char* ?

> Also I wonder if this is mixing two issues. a) provide more information
> (the tried path/file) when something goes wrong and b) providing a
> "chain of errors" to explain why some call really failed when the
> operation was really a compounded search result. So do we really need
> just a string (or a list of strings)? Or do we need two separate things.
> A way to provide extra (dynamic) detail when signaling a dwfl error. And
> a way to chain multiple errors in case a function needs to report
> multiple failure results when an operation failed to show everything
> that was tried?

I'm not 100% sure what you mean exactly here re. chaining errors. Do you
mean specifically for compound operations, the user should be able to
easily have programmatic access to the different operations that took
place (along with their respective errors)?

E.g. a linked list of struct { char* operation; char* errmsg } ?

Or do you mean that it should be easy in the elfutils codebase to "append"
to the error message rather than having to accumulate the whole message
before doing __libdw_seterrno_details(). In that case, we could get away
with just adding a __libdw_append_details().

> There is also the detail that not all lookups need to be path/file based
> (find_elf and find_debuginfo aren't properly documented unfortunately).
> But the lookups don't need to create the Elf or Dwarf from an actual
> file (name). They can also create them from a file descriptor. Or even
> create an Elf image "out of thin air". Which find_elf might actually do
> by pulling the image from remote memory of the process when it cannot be
> found on disk. And I can imagine a find_debuginfo callback that gets
> debuginfo files from some remote server.

Right, the actual usage of the dynamic error message capabilities is of
course on a case-by-case basis. What I added in patch 4 was just to
address the common case. But we can use this for other types of errors in
other places.

Thanks,

Jonathan