* [PATCH 0/2] addr2line: new option -n to add a newline at the end @ 2022-12-19 13:53 Maurizio Papini 2022-12-19 13:53 ` [PATCH 1/2] " Maurizio Papini ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Maurizio Papini @ 2022-12-19 13:53 UTC (permalink / raw) To: binutils; +Cc: Maurizio Papini This series adds a new option to addr2line (-n) to append a newline after the last informative one. This new option is helpful for using a running addr2line process and performing queries, in particular when the option -i is requested: the additional empty line can be used to mark the end of the inlined functions lists so that an application can get the output without defining a timeout. The first patch adds the new option while the second one adds the relative test. Let me know what you think. Maurizio Maurizio Papini (2): addr2line: new option -n to add a newline at the end addr2line: test to check -n option binutils/addr2line.c | 11 +++++++++-- binutils/doc/binutils.texi | 5 +++++ binutils/testsuite/binutils-all/addr2line.exp | 10 ++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) -- 2.38.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] addr2line: new option -n to add a newline at the end 2022-12-19 13:53 [PATCH 0/2] addr2line: new option -n to add a newline at the end Maurizio Papini @ 2022-12-19 13:53 ` Maurizio Papini 2022-12-19 13:53 ` [PATCH 2/2] addr2line: test to check -n option Maurizio Papini 2023-01-09 16:04 ` [PATCH 0/2] addr2line: new option -n to add a newline at the end Maurizio Papini 2 siblings, 0 replies; 9+ messages in thread From: Maurizio Papini @ 2022-12-19 13:53 UTC (permalink / raw) To: binutils; +Cc: Maurizio Papini --- binutils/addr2line.c | 11 +++++++++-- binutils/doc/binutils.texi | 5 +++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/binutils/addr2line.c b/binutils/addr2line.c index baf16716182..d9338e9f1ea 100644 --- a/binutils/addr2line.c +++ b/binutils/addr2line.c @@ -45,7 +45,7 @@ static bool with_functions; /* -f, show function names. */ static bool do_demangle; /* -C, demangle names. */ static bool pretty_print; /* -p, print on one line. */ static bool base_names; /* -s, strip directory names. */ - +static bool add_newline; /* -n, add a newline at the end. */ /* Flags passed to the name demangler. */ static int demangle_flags = DMGL_PARAMS | DMGL_ANSI; @@ -70,6 +70,7 @@ static struct option long_options[] = {"no-recursion-limit", no_argument, NULL, 'r'}, {"section", required_argument, NULL, 'j'}, {"target", required_argument, NULL, 'b'}, + {"add-newline", no_argument, NULL, 'n'}, {"help", no_argument, NULL, 'H'}, {"version", no_argument, NULL, 'V'}, {0, no_argument, 0, 0} @@ -102,6 +103,7 @@ usage (FILE *stream, int status) -C --demangle[=style] Demangle function names\n\ -R --recurse-limit Enable a limit on recursion whilst demangling. [Default]\n\ -r --no-recurse-limit Disable a limit on recursion whilst demangling\n\ + -n --add-newline Add a newline at the end of the output\n\ -h --help Display this information\n\ -v --version Display the program's version\n\ \n")); @@ -417,6 +419,8 @@ translate_addresses (bfd *abfd, asection *section) } } + if(add_newline) + printf("\n"); /* fflush() is essential for using this command as a server child process that reads addresses from a pipe and responds with line number information, processing one address at a @@ -505,7 +509,7 @@ main (int argc, char **argv) file_name = NULL; section_name = NULL; target = NULL; - while ((c = getopt_long (argc, argv, "ab:Ce:rRsfHhij:pVv", long_options, (int *) 0)) + while ((c = getopt_long (argc, argv, "ab:Ce:rRsfnHhij:pVv", long_options, (int *) 0)) != EOF) { switch (c) @@ -564,6 +568,9 @@ main (int argc, char **argv) case 'j': section_name = optarg; break; + case 'n': + add_newline = true; + break; default: usage (stderr, 1); break; diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi index c9d52d7c92f..0940708d77e 100644 --- a/binutils/doc/binutils.texi +++ b/binutils/doc/binutils.texi @@ -3976,6 +3976,7 @@ addr2line [@option{-a}|@option{--addresses}] [@option{-f}|@option{--functions}] [@option{-s}|@option{--basename}] [@option{-i}|@option{--inlines}] [@option{-p}|@option{--pretty-print}] + [@option{-n}|@option{--add-newline}] [@option{-j}|@option{--section=}@var{name}] [@option{-H}|@option{--help}] [@option{-V}|@option{--version}] [addr addr @dots{}] @@ -4102,6 +4103,10 @@ Make the output more human friendly: each location are printed on one line. If option @option{-i} is specified, lines for all enclosing scopes are prefixed with @samp{(inlined by)}. +@item -n +@itemx --add-newline +Add a newline at the end of the output. + @item -r @itemx -R @itemx --recurse-limit -- 2.38.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] addr2line: test to check -n option 2022-12-19 13:53 [PATCH 0/2] addr2line: new option -n to add a newline at the end Maurizio Papini 2022-12-19 13:53 ` [PATCH 1/2] " Maurizio Papini @ 2022-12-19 13:53 ` Maurizio Papini 2023-01-09 16:04 ` [PATCH 0/2] addr2line: new option -n to add a newline at the end Maurizio Papini 2 siblings, 0 replies; 9+ messages in thread From: Maurizio Papini @ 2022-12-19 13:53 UTC (permalink / raw) To: binutils; +Cc: Maurizio Papini --- binutils/testsuite/binutils-all/addr2line.exp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/binutils/testsuite/binutils-all/addr2line.exp b/binutils/testsuite/binutils-all/addr2line.exp index 957ae55df33..59c0924412c 100644 --- a/binutils/testsuite/binutils-all/addr2line.exp +++ b/binutils/testsuite/binutils-all/addr2line.exp @@ -61,6 +61,16 @@ if ![regexp -line "^(\[0-9a-fA-F\]+)? +\[Tt\] ${dot}fn" $output contents] then { pass "$testname -f option" } +#testcase for -n option: check the extra newline +#Using the same fn function address used in -f option. + set got [binutils_run $ADDR2LINE "-n -f -e tmpdir/testprog$exe [lindex $list 0]"] + set want "fn\n$srcdir/$subdir/testprog.c:\[0-9\]+\n\n" + if ![regexp $want $got] then { + fail "$testname -fn option $got\n" + } else { + pass "$testname -fn option" + } + #testcase for -s option. #Using the same fn function address used in -f option. set got [binutils_run $ADDR2LINE "-s -e tmpdir/testprog$exe [lindex $list 0]"] -- 2.38.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] addr2line: new option -n to add a newline at the end 2022-12-19 13:53 [PATCH 0/2] addr2line: new option -n to add a newline at the end Maurizio Papini 2022-12-19 13:53 ` [PATCH 1/2] " Maurizio Papini 2022-12-19 13:53 ` [PATCH 2/2] addr2line: test to check -n option Maurizio Papini @ 2023-01-09 16:04 ` Maurizio Papini 2023-01-09 16:17 ` Jan Beulich 2023-01-10 12:19 ` Nick Clifton 2 siblings, 2 replies; 9+ messages in thread From: Maurizio Papini @ 2023-01-09 16:04 UTC (permalink / raw) To: binutils [-- Attachment #1: Type: text/plain, Size: 1120 bytes --] Hi All, Do you think this should go through an RFC? Do you have any thoughts? Thanks for your time. BR, Maurizio On Mon, Dec 19, 2022 at 2:53 PM Maurizio Papini <mpapini@redhat.com> wrote: > This series adds a new option to addr2line (-n) to append a newline > after the last informative one. > > This new option is helpful for using a running addr2line process and > performing queries, in particular when the option -i is requested: the > additional empty line can be used to mark the end of the inlined functions > lists so that an application can get the output without defining a timeout. > > The first patch adds the new option while the second one adds the relative > test. > > Let me know what you think. > > Maurizio > > > Maurizio Papini (2): > addr2line: new option -n to add a newline at the end > addr2line: test to check -n option > > binutils/addr2line.c | 11 +++++++++-- > binutils/doc/binutils.texi | 5 +++++ > binutils/testsuite/binutils-all/addr2line.exp | 10 ++++++++++ > 3 files changed, 24 insertions(+), 2 deletions(-) > > -- > 2.38.1 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] addr2line: new option -n to add a newline at the end 2023-01-09 16:04 ` [PATCH 0/2] addr2line: new option -n to add a newline at the end Maurizio Papini @ 2023-01-09 16:17 ` Jan Beulich 2023-01-10 14:17 ` Maurizio Papini 2023-01-10 12:19 ` Nick Clifton 1 sibling, 1 reply; 9+ messages in thread From: Jan Beulich @ 2023-01-09 16:17 UTC (permalink / raw) To: Maurizio Papini; +Cc: binutils On 09.01.2023 17:04, Maurizio Papini via Binutils wrote: > Do you think this should go through an RFC? Do you have any thoughts? Well, I'm of two minds here, which so far kept me from responding: On one hand an option that's useful to someone is probably a good thing. Otoh an option to control a single newline character seems a little too fine grained to me. Plus I'm a little concerned of burning a short option for this (niche?) issue. Since you say it's specifically an issue with -i, would it be an option to add a long-only option providing the intended variant of behavior, i.e. combining what would (aiui) be "-i -n" with the current proposal? Jan > On Mon, Dec 19, 2022 at 2:53 PM Maurizio Papini <mpapini@redhat.com> wrote: > >> This series adds a new option to addr2line (-n) to append a newline >> after the last informative one. >> >> This new option is helpful for using a running addr2line process and >> performing queries, in particular when the option -i is requested: the >> additional empty line can be used to mark the end of the inlined functions >> lists so that an application can get the output without defining a timeout. >> >> The first patch adds the new option while the second one adds the relative >> test. >> >> Let me know what you think. >> >> Maurizio >> >> >> Maurizio Papini (2): >> addr2line: new option -n to add a newline at the end >> addr2line: test to check -n option >> >> binutils/addr2line.c | 11 +++++++++-- >> binutils/doc/binutils.texi | 5 +++++ >> binutils/testsuite/binutils-all/addr2line.exp | 10 ++++++++++ >> 3 files changed, 24 insertions(+), 2 deletions(-) >> >> -- >> 2.38.1 >> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] addr2line: new option -n to add a newline at the end 2023-01-09 16:17 ` Jan Beulich @ 2023-01-10 14:17 ` Maurizio Papini 2023-01-10 15:50 ` Jan Beulich 0 siblings, 1 reply; 9+ messages in thread From: Maurizio Papini @ 2023-01-10 14:17 UTC (permalink / raw) To: Jan Beulich; +Cc: binutils [-- Attachment #1: Type: text/plain, Size: 2328 bytes --] Thanks for your feedback Jan. Yes, I understand that one option for a single newline smells fulsome but the idea is to use it to mark the end of output when addr2line is used as a "server" process: right now I see only the "-i" use case, but there could be others in the future for new options. Said that I would propose adding the "add-newline" long option only to trivially add a new line, that could be reused for other output. What do you think? Maurizio On Mon, Jan 9, 2023 at 5:17 PM Jan Beulich <jbeulich@suse.com> wrote: > On 09.01.2023 17:04, Maurizio Papini via Binutils wrote: > > Do you think this should go through an RFC? Do you have any thoughts? > > Well, I'm of two minds here, which so far kept me from responding: On one > hand an option that's useful to someone is probably a good thing. Otoh an > option to control a single newline character seems a little too fine > grained to me. Plus I'm a little concerned of burning a short option for > this (niche?) issue. Since you say it's specifically an issue with -i, > would it be an option to add a long-only option providing the intended > variant of behavior, i.e. combining what would (aiui) be "-i -n" with the > current proposal? > > Jan > > > On Mon, Dec 19, 2022 at 2:53 PM Maurizio Papini <mpapini@redhat.com> > wrote: > > > >> This series adds a new option to addr2line (-n) to append a newline > >> after the last informative one. > >> > >> This new option is helpful for using a running addr2line process and > >> performing queries, in particular when the option -i is requested: the > >> additional empty line can be used to mark the end of the inlined > functions > >> lists so that an application can get the output without defining a > timeout. > >> > >> The first patch adds the new option while the second one adds the > relative > >> test. > >> > >> Let me know what you think. > >> > >> Maurizio > >> > >> > >> Maurizio Papini (2): > >> addr2line: new option -n to add a newline at the end > >> addr2line: test to check -n option > >> > >> binutils/addr2line.c | 11 +++++++++-- > >> binutils/doc/binutils.texi | 5 +++++ > >> binutils/testsuite/binutils-all/addr2line.exp | 10 ++++++++++ > >> 3 files changed, 24 insertions(+), 2 deletions(-) > >> > >> -- > >> 2.38.1 > >> > >> > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] addr2line: new option -n to add a newline at the end 2023-01-10 14:17 ` Maurizio Papini @ 2023-01-10 15:50 ` Jan Beulich 0 siblings, 0 replies; 9+ messages in thread From: Jan Beulich @ 2023-01-10 15:50 UTC (permalink / raw) To: Maurizio Papini; +Cc: binutils On 10.01.2023 15:17, Maurizio Papini wrote: > Thanks for your feedback Jan. > > Yes, I understand that one option for a single newline smells fulsome but > the > idea is to use it to mark the end of output when addr2line is used as a > "server" > process: right now I see only the "-i" use case, but there could be others > in the > future for new options. Said that I would propose adding the "add-newline" > long > option only to trivially add a new line, that could be reused for other > output. > What do you think? I don't really like the idea, but I can see this being a compromise to cover your use case. Yet in your reply to Nick you indicate you might go the route he proposed, which I understand would allow us to get away without any such option. If, however, an option is still needed from your pov, then please make its name more meaningful than just "add-newline" - that doesn't tell at all what the real purpose is. Maybe "separate-<whatever>", with <whatever> suitably replaced, and then possibly also (in the future, not necessarily right now) allowing to specify a character to use as the separator in place of the (default) newline. Jan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] addr2line: new option -n to add a newline at the end 2023-01-09 16:04 ` [PATCH 0/2] addr2line: new option -n to add a newline at the end Maurizio Papini 2023-01-09 16:17 ` Jan Beulich @ 2023-01-10 12:19 ` Nick Clifton 2023-01-10 15:28 ` Maurizio Papini 1 sibling, 1 reply; 9+ messages in thread From: Nick Clifton @ 2023-01-10 12:19 UTC (permalink / raw) To: Maurizio Papini, binutils Hi Maurizio, > Do you think this should go through an RFC? Do you have any thoughts? I am sorry - I totally missed this patch submission. :-( >> This series adds a new option to addr2line (-n) to append a newline >> after the last informative one. I have to say that I am kind of with Jan on this one - it seems like a lot of effort to fix a small problem. Isn't there an easier way that would not involve patching addr2line ? For example if you use the -a/--addresses option then each decoded address will be prefixed with a line containing a simple hex number. This would allow a consumer of addr2line's output to detect the start of each decoded address. > the > additional empty line can be used to mark the end of the inlined functions > lists so that an application can get the output without defining a timeout. I am not sure what you are getting at here. Are you saying that addr2line can get stuck and so a timeout is needed in order to be to distinguish between addr2line being slow and addr2line being stuck ? If so then this sounds like a more serious problem that needs to be addressed by something other than just adding a blank line to the output. Cheers Nick ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] addr2line: new option -n to add a newline at the end 2023-01-10 12:19 ` Nick Clifton @ 2023-01-10 15:28 ` Maurizio Papini 0 siblings, 0 replies; 9+ messages in thread From: Maurizio Papini @ 2023-01-10 15:28 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils Hi Nick and thanks for your time. > I am sorry - I totally missed this patch submission. :-( The patch is definitely not urgent, trivial stuff indeed, and my submission day was so close to the end of 2022. > I am not sure what you are getting at here. Are you saying that addr2line > can get stuck and so a timeout is needed in order to be to distinguish between > addr2line being slow and addr2line being stuck ? If so then this sounds > like a more serious problem that needs to be addressed by something other > than just adding a blank line to the output. No, I didn't see this kind of problem. I'm talking about this use case: addr2line as a process, with "-i" option, that is reading from stdin working on a big binary, say like the Linux kernel with debug info (~400M+), so that spawning the process once is definitely better than doing it every time. > I have to say that I am kind of with Jan on this one - it seems like > a lot of effort to fix a small problem. Isn't there an easier way > that would not involve patching addr2line ? For example if you use > the -a/--addresses option then each decoded address will be prefixed > with a line containing a simple hex number. This would allow a consumer > of addr2line's output to detect the start of each decoded address. I'm thinking about using your suggestion (thanks): with -a the address is added as a prefix as you said, while it would have been useful to have it at the end :-), but querying two times... Maurizio Maurizio On Tue, Jan 10, 2023 at 1:19 PM Nick Clifton <nickc@redhat.com> wrote: > > Hi Maurizio, > > > Do you think this should go through an RFC? Do you have any thoughts? > > I am sorry - I totally missed this patch submission. :-( > > >> This series adds a new option to addr2line (-n) to append a newline > >> after the last informative one. > > I have to say that I am kind of with Jan on this one - it seems like > a lot of effort to fix a small problem. Isn't there an easier way > that would not involve patching addr2line ? For example if you use > the -a/--addresses option then each decoded address will be prefixed > with a line containing a simple hex number. This would allow a consumer > of addr2line's output to detect the start of each decoded address. > > > > the > > additional empty line can be used to mark the end of the inlined functions > > lists so that an application can get the output without defining a timeout. > > I am not sure what you are getting at here. Are you saying that addr2line > can get stuck and so a timeout is needed in order to be to distinguish between > addr2line being slow and addr2line being stuck ? If so then this sounds > like a more serious problem that needs to be addressed by something other > than just adding a blank line to the output. > > Cheers > Nick > > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-01-10 15:50 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-19 13:53 [PATCH 0/2] addr2line: new option -n to add a newline at the end Maurizio Papini 2022-12-19 13:53 ` [PATCH 1/2] " Maurizio Papini 2022-12-19 13:53 ` [PATCH 2/2] addr2line: test to check -n option Maurizio Papini 2023-01-09 16:04 ` [PATCH 0/2] addr2line: new option -n to add a newline at the end Maurizio Papini 2023-01-09 16:17 ` Jan Beulich 2023-01-10 14:17 ` Maurizio Papini 2023-01-10 15:50 ` Jan Beulich 2023-01-10 12:19 ` Nick Clifton 2023-01-10 15:28 ` Maurizio Papini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).