Re: [PATCH 0/4] Improve elfutils diagnostics

[Mark Wielaard <mjw at redhat dot com>]

Hi Jonathan,

Sorry for the brief reply, I am somewhat offline today, but wanted to
give some quick feedback. Apologies if it is too brief or not too well
thought out. In that case I'll take more time to think things through
tomorrow.

On Wed, May 13, 2015 at 02:44:50PM -0400, Jonathan Lebon wrote:
> > 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.

Yes, lets make sure the interface works for all of them.

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

Yes, we might want to provide such a function and document that if the
callback returns failure it might have called that "detailed_failures"
functions a couple of times to provide extra feedback.

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

See also my message to Frank. I think we should combine these two things.
So a "detail" is an error code plus message (e.g. file name), that the
user can combine, call printf (stderr, "No Dwarf: %s %s\n",
			       dwfl_errmsg (d.err), d.detail);
And there is an int dwfl_details (Dwfl_Detail **details) function that
the user can call to get the number of details there is as an array.
Or maybe even as an opaque type that the user iterates over extracting
the err and msg with a separate function.

Cheers,

Mark