1. Patchew
  2. linux
  3. View series

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

Breno Leitao posted 1 patch 1 month, 3 weeks ago
There is a newer version of this series
scripts/decode_stacktrace.sh | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[PATCH] scripts/decode_stacktrace.sh: remove trailing space
Posted by Breno Leitao 1 month, 3 weeks ago 
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
Posted by Elliot Berman 1 month, 2 weeks ago 
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
>
Re: [PATCH] scripts/decode_stacktrace.sh: remove trailing space
Posted by Carlos Llamas 1 month, 3 weeks ago 
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
Posted by Breno Leitao 1 month, 2 weeks ago 
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
Posted by Carlos Llamas 1 month, 2 weeks ago 
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
Posted by Stephen Boyd 1 month, 3 weeks ago 
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>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.