Hi Jan,
On Wed, 2013-10-30 at 14:21 +0100, Jan Kratochvil wrote:
> And just print the adjusted pc directly by default (or
> pc if you want to match what p/gstack does). That IMHO gives the output
> users would expect (and is easier to parse).
pc_adjusted really cannot be printed, just cannot, it points nowhere, it is
incompatible with everything etc.
Well, it is the pc value we use to lookup the symbol. Which might or
might not be the exact "call address". But yes, I do see your point.
Using pc is better because it is a good known (return) address.
So far I have kept the "- 1" output there. elfutils do not
completely conform
character-by-character to any existing utility and neither does eu-stack in
other parts of the output.
I find that " - 1" part very useful for users in the output, when looking at
objdump -d etc. And it can be safely ignored if nobody understands that.
If you object to " - 1" one could print for example " (call)" there
instead.
Now that addresses are all of the same length I don't find it as
distracting, since it is always put in the same place. Lets keep it.
> Keep it simple by default.
elfutils should be useful, dropping useful information to make it more close
to existing utilities does not seem correct to me.
> You could add an extra flag to print more information about a frame,
> like whether or not it is an activation, but I don't think that should
> be the default.
If there is no other choice then at least some -v flag would be needed.
I do want as much information as we can give. But IMHO the default
should be as basic as possible. Lets do introduce a -v flag (or some
others for symbol name demangling, module names, source/line, function
arguments, etc.). But not right now. Lets first get the basic
functionality in. I am very eager to finally see this committed!
Used. Although s/wide/width/, that was IMO a typo, wasn't it?
Yeah, my English is not always as good as it should be...
One tiny formatting nitpick (sorry):
+ printf ("#%-2u 0x%0*" PRIx64 "%4s\t%s\n",
(*framenop)++, width, (uint64_t) pc,
! isactivation ? "- 1" : "", symname);
Could we replace the \t with a simple space?
Now that the output lines are all of equal width a space is enough IMHO.
With or without that change I think it is good to go in.
Thanks,
Mark