Re: [PATCH 0/4] Improve elfutils diagnostics

[Mark Wielaard <mjw@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2910 bytes --]

Hi Jonathan,

On Mon, 2015-05-11 at 15:38 -0400, Jonathan Lebon wrote:
> This patch series attempts to improve elfutils feedback to help guide
> the user towards fixing erroneous situations.

Thanks, this is really helpful.

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

> The third patch add the new function dwfl_errmsg_details(), which can be
> used to provide dynamic freeform information to the user to supplement
> the static error message from dwfl_errmsg().
> 
> The fourth patch makes use of this facility to provide the user with all
> the paths that were attempted while looking for the debug file.

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. getdwarf and
getelf both allow the user to plug in their own lookup callbacks to make
things things even more interesting.

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.

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?

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.

Cheers,

Mark