[PATCH] scripts/decode_stacktrace.sh: remove trailing space

[PATCH] scripts/decode_stacktrace.sh: remove trailing space

[Breno Leitao]

decode_stacktrace.sh adds a trailing space at the end of the decoded
stack if the module is not set (in most of the lines), which makes the
some lines of the stack having trailing space and some others not.

Do not add an extra space at the end of the line if module is not set,
adding consistency in output formatting.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 scripts/decode_stacktrace.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
index 826836d264c6..4b3502a007fd 100755
--- a/scripts/decode_stacktrace.sh
+++ b/scripts/decode_stacktrace.sh
@@ -311,7 +311,12 @@ handle_line() {
 	parse_symbol # modifies $symbol
 
 	# Add up the line number to the symbol
-	echo "${words[@]}" "$symbol $module"
+	if [ -z ${module} ]
+	then
+		echo "${words[@]}" "$symbol"
+	else
+		echo "${words[@]}" "$symbol $module"
+	fi
 }
 
 while read line; do
-- 
2.43.5

Re: [PATCH] scripts/decode_stacktrace.sh: remove trailing space

[Stephen Boyd]

Quoting Breno Leitao (2024-10-03 03:30:05)
> decode_stacktrace.sh adds a trailing space at the end of the decoded
> stack if the module is not set (in most of the lines), which makes the
> some lines of the stack having trailing space and some others not.
>
> Do not add an extra space at the end of the line if module is not set,
> adding consistency in output formatting.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH] scripts/decode_stacktrace.sh: remove trailing space

[Carlos Llamas]

On Thu, Oct 03, 2024 at 03:30:05AM -0700, Breno Leitao wrote:
> decode_stacktrace.sh adds a trailing space at the end of the decoded
> stack if the module is not set (in most of the lines), which makes the
> some lines of the stack having trailing space and some others not.
> 
> Do not add an extra space at the end of the line if module is not set,
> adding consistency in output formatting.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  scripts/decode_stacktrace.sh | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
> index 826836d264c6..4b3502a007fd 100755
> --- a/scripts/decode_stacktrace.sh
> +++ b/scripts/decode_stacktrace.sh
> @@ -311,7 +311,12 @@ handle_line() {
>  	parse_symbol # modifies $symbol
>  
>  	# Add up the line number to the symbol
> -	echo "${words[@]}" "$symbol $module"
> +	if [ -z ${module} ]
> +	then

nit: it seems the convention would have been something like:
	if [[ -z $module ]]; then

> +		echo "${words[@]}" "$symbol"
> +	else
> +		echo "${words[@]}" "$symbol $module"

I guess the space could have also been prepended in module above? e.g
	module=" $module"

> +	fi
>  }
>  
>  while read line; do
> -- 
> 2.43.5
> 

...either way this looks good to me.

Reviewed-by: Carlos Llamas <cmllamas@google.com>

Re: [PATCH] scripts/decode_stacktrace.sh: remove trailing space

[Breno Leitao]

Hello Carlos,

On Mon, Oct 07, 2024 at 07:29:27PM +0000, Carlos Llamas wrote:
> On Thu, Oct 03, 2024 at 03:30:05AM -0700, Breno Leitao wrote:
> > decode_stacktrace.sh adds a trailing space at the end of the decoded
> > stack if the module is not set (in most of the lines), which makes the
> > some lines of the stack having trailing space and some others not.
> > 
> > Do not add an extra space at the end of the line if module is not set,
> > adding consistency in output formatting.
> > 
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> > ---
> >  scripts/decode_stacktrace.sh | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
> > index 826836d264c6..4b3502a007fd 100755
> > --- a/scripts/decode_stacktrace.sh
> > +++ b/scripts/decode_stacktrace.sh
> > @@ -311,7 +311,12 @@ handle_line() {
> >  	parse_symbol # modifies $symbol
> >  
> >  	# Add up the line number to the symbol
> > -	echo "${words[@]}" "$symbol $module"
> > +	if [ -z ${module} ]
> > +	then
> 
> nit: it seems the convention would have been something like:
> 	if [[ -z $module ]]; then

That is how I wrote originally, in fact, but, the rest of the code is
using the other way. Example:

        if [ -z $release ] ; then
                release=$(gdb -ex 'print init_uts_ns.name.release' -ex 'quit' -quiet -batch "$vmlinux" 2>/dev/null | sed -n 's/\$1 = "\(.*\)".*/\1/p')
        fi
        if [ -n "${release}" ] ; then
                release_dirs="/usr/lib/debug/lib/modules/$release /lib/modules/$release"
        fi

Thanks for the review,
Breno

Re: [PATCH] scripts/decode_stacktrace.sh: remove trailing space

[Carlos Llamas]

On Tue, Oct 08, 2024 at 12:18:29PM +0100, Breno Leitao wrote:
> Hello Carlos,
> 
> On Mon, Oct 07, 2024 at 07:29:27PM +0000, Carlos Llamas wrote:
> > On Thu, Oct 03, 2024 at 03:30:05AM -0700, Breno Leitao wrote:
> > > decode_stacktrace.sh adds a trailing space at the end of the decoded
> > > stack if the module is not set (in most of the lines), which makes the
> > > some lines of the stack having trailing space and some others not.
> > > 
> > > Do not add an extra space at the end of the line if module is not set,
> > > adding consistency in output formatting.
> > > 
> > > Signed-off-by: Breno Leitao <leitao@debian.org>
> > > ---
> > >  scripts/decode_stacktrace.sh | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
> > > index 826836d264c6..4b3502a007fd 100755
> > > --- a/scripts/decode_stacktrace.sh
> > > +++ b/scripts/decode_stacktrace.sh
> > > @@ -311,7 +311,12 @@ handle_line() {
> > >  	parse_symbol # modifies $symbol
> > >  
> > >  	# Add up the line number to the symbol
> > > -	echo "${words[@]}" "$symbol $module"
> > > +	if [ -z ${module} ]
> > > +	then
> > 
> > nit: it seems the convention would have been something like:
> > 	if [[ -z $module ]]; then
> 
> That is how I wrote originally, in fact, but, the rest of the code is
> using the other way. Example:
> 
>         if [ -z $release ] ; then

This is actually problematic if $release can have any spaces but I
suspect it's not possible here. I'd still either quote "$release" or use
double brackets e.g. [[ -z $release ]] to be safe.

BTW, I assume $module can't have any spaces either?

>                 release=$(gdb -ex 'print init_uts_ns.name.release' -ex 'quit' -quiet -batch "$vmlinux" 2>/dev/null | sed -n 's/\$1 = "\(.*\)".*/\1/p')
>         fi
>         if [ -n "${release}" ] ; then

Yeah, there is no need for the curly braces here, they are not doing
anything.

>                 release_dirs="/usr/lib/debug/lib/modules/$release /lib/modules/$release"
>         fi
> 
> Thanks for the review,
> Breno

Note that I was initally referring to the 'then' being on a new line but
these are all minor nits.

Cheers,
Carlos Llamas

Re: [PATCH] scripts/decode_stacktrace.sh: remove trailing space

[Elliot Berman]

On Thu, Oct 03, 2024 at 03:30:05AM -0700, Breno Leitao wrote:
> decode_stacktrace.sh adds a trailing space at the end of the decoded
> stack if the module is not set (in most of the lines), which makes the
> some lines of the stack having trailing space and some others not.
> 
> Do not add an extra space at the end of the line if module is not set,
> adding consistency in output formatting.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  scripts/decode_stacktrace.sh | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
> index 826836d264c6..4b3502a007fd 100755
> --- a/scripts/decode_stacktrace.sh
> +++ b/scripts/decode_stacktrace.sh
> @@ -311,7 +311,12 @@ handle_line() {
>  	parse_symbol # modifies $symbol
>  
>  	# Add up the line number to the symbol
> -	echo "${words[@]}" "$symbol $module"
> +	if [ -z ${module} ]

After adding the quoting as mentioned by Carlos,

Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>

> +	then
> +		echo "${words[@]}" "$symbol"
> +	else
> +		echo "${words[@]}" "$symbol $module"
> +	fi
>  }
>  
>  while read line; do
> -- 
> 2.43.5
>