On Mon, 2013-12-16 at 15:55 +0900, Masatake YAMATO wrote: > This patch added the binary file name where the function is > defined when -v option is given, and the source file name > where the function is defined when -v -v options are given.
Cool. I like this idea. But I think it would be better to have separate options so the user can precisely say what they want. So one -m, --modules option to add the module name and one -s, --source option to add the source name. -v, --verbose would then always be the most verbose output possible (and enable all extra info). > This is based on private discussion with Jan Kratochvil > <[email protected]>. > > Signed-off-by: Masatake YAMATO <[email protected]> > --- > src/ChangeLog | 9 +++++++++ > src/stack.c | 26 ++++++++++++++++++++++---- > 2 files changed, 31 insertions(+), 4 deletions(-) > > diff --git a/src/ChangeLog b/src/ChangeLog > index f899858..3bf2426 100644 > --- a/src/ChangeLog > +++ b/src/ChangeLog > @@ -1,3 +1,12 @@ > +2013-12-16 Masatake YAMATO <[email protected]> > + > + * stack.c (verbose): Change the type to int. > + (parse_opt): Increment verbose for each time when > + '-v' is found. > + (main): Added '-v' to the help message. > + (frame_callback): Print binary and source file name > + if verbose > 0. > + > 2013-11-25 Petr Machata <[email protected]> > > * elflint.c (valid_e_machine): Add EM_AARCH64. > diff --git a/src/stack.c b/src/stack.c > index f428ed0..edb479f 100644 > --- a/src/stack.c > +++ b/src/stack.c > @@ -36,7 +36,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = print_version; > /* Bug report address. */ > ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT; > > -static bool verbose = false; > +static int verbose = 0; > > static int > frame_callback (Dwfl_Frame *state, void *arg) > @@ -78,7 +78,25 @@ frame_callback (Dwfl_Frame *state, void *arg) > printf ("#%-2u 0x%0*" PRIx64, (*framenop)++, width, (uint64_t) pc); > if (verbose) > printf ("%4s", ! isactivation ? "- 1" : ""); > - printf (" %s\n", symname); > + printf (" %s", symname); > + if (verbose > 0) > + { > + const char* fname; > + Dwfl_Line * lineobj; > + int line, col; > + const char* sname; > + > + fname = dwfl_module_info(mod, NULL, NULL, NULL, NULL, NULL, NULL, > NULL); > + printf (" - %s", fname? fname: "?"); There are cases where dwfl_module_info returns the empty string "". In that case it is nice to provide the basename of the mainfile argument returned by dwfl_module_info instead. > + if (verbose > 1) > + { > + lineobj = dwfl_module_getsrc(mod, pc_adjusted); > + line = col = -1; > + sname = lineobj? (dwfl_lineinfo(lineobj, NULL, &line, &col, NULL, > NULL)?: "?"): "?"; > + printf(" (%s:%d:%d)", sname, line, col); > + } Since line and/or col can be zero or minus one when they don't exist I don't think you should print them then. Only print the sname:line when line <= 0 and only print sname:line:col when col <= 0. You could also look at how the src/addr2line.c print_src function formats source lines, but that would introduce two more options (basename and compdir), which might or might not be overkill. Thanks, Mark
