[0/2] addr2line: new option -n to add a newline at the end

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

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

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

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
  

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

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

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