[PATCH V2] tools: mm: Added modern version of shell quote and a stanza

Bhaskar Chowdhury posted 1 patch 3 months, 1 week ago
tools/mm/slabinfo-gnuplot.sh | 44 ++++++++++++++++++++++++------------
1 file changed, 30 insertions(+), 14 deletions(-)
[PATCH V2] tools: mm: Added modern version of shell quote and a stanza
Posted by Bhaskar Chowdhury 3 months, 1 week ago
Three things precisely:

Replaced backquote with dollar-parentheses ...that is modern way of quoting in
shell script. Improved readability.

Added a stranza of missing/required command for the operation,in this case
gnuplot and that too in shell path.

And lastly,use "command -v" to search the command i.e gnuplot and the REASON
for that :

The -v option tell show shell will invoke the command specified as its
options.Basically to avoid dependency on something outside of the shell.It is
also execute command found in the PATH. Essentially, ignoring other similar
name stuff curated somewhere else in the system.

Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
---
 V2: What changes from V1: Verbose changelog, especially using command -v to
 find the specific program i.e. gnuplot.

 tools/mm/slabinfo-gnuplot.sh | 44 ++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/tools/mm/slabinfo-gnuplot.sh b/tools/mm/slabinfo-gnuplot.sh
index 873a892147e5..6c9add4bb8ad 100644
--- a/tools/mm/slabinfo-gnuplot.sh
+++ b/tools/mm/slabinfo-gnuplot.sh
@@ -46,6 +46,22 @@ check_file_exist()
 	fi
 }

+# This variable could have space separated value
+my_needed_commands="gnuplot"
+
+missing_counter=0
+for needed_command in $my_needed_commands; do
+  if ! hash "$needed_command" >/dev/null 2>&1; then
+    printf "Command not found in PATH: %s\n" "$needed_command" >&2
+    ((missing_counter++))
+  fi
+done
+
+if ((missing_counter > 0)); then
+  printf "Minimum %d commands are missing in PATH, aborting\n" "$missing_counter" >&2
+  exit 1
+fi
+
 do_slabs_plotting()
 {
 	local file=$1
@@ -58,13 +74,13 @@ do_slabs_plotting()

 	check_file_exist "$file"

-	out_file=`basename "$file"`
+	out_file=$(basename "$file")
 	if [ $xmax -ne 0 ]; then
 		range="$range::$xmax"
 		lines=$((xmax-xmin))
 	fi

-	wc_lines=`cat "$file" | wc -l`
+	wc_lines=$(cat "$file" | wc -l)
 	if [ $? -ne 0 ] || [ "$wc_lines" -eq 0 ] ; then
 		wc_lines=$lines
 	fi
@@ -78,7 +94,7 @@ do_slabs_plotting()
 		xtic_rotate=90
 	fi

-gnuplot -p << EOF
+$(command -v gnuplot) -p << EOF
 #!/usr/bin/env gnuplot

 set terminal png enhanced size $width,$height large
@@ -113,14 +129,14 @@ do_totals_plotting()
 	for i in "${t_files[@]}"; do
 		check_file_exist "$i"

-		file="$file"`basename "$i"`
+		file="$file"$(basename "$i")
 		gnuplot_cmd="$gnuplot_cmd '$i' $range using 1 title\
 			'$i Memory usage' with lines,"
 		gnuplot_cmd="$gnuplot_cmd '' $range using 2 title \
 			'$i Loss' with lines,"
 	done

-gnuplot -p << EOF
+$(command -v gnuplot) -p << EOF
 #!/usr/bin/env gnuplot

 set terminal png enhanced size $width,$height large
@@ -148,26 +164,26 @@ do_preprocess()

 	# use only 'TOP' slab (biggest memory usage or loss)
 	let lines=3
-	out=`basename "$in"`"-slabs-by-loss"
-	`cat "$in" | grep -A "$lines" 'Slabs sorted by loss' |\
+	out=$(basename "$in")"-slabs-by-loss"
+	$(cat "$in" | grep -A "$lines" 'Slabs sorted by loss' |\
 		grep -E -iv '\-\-|Name|Slabs'\
-		| awk '{print $1" "$4+$2*$3" "$4}' > "$out"`
+		| awk '{print $1" "$4+$2*$3" "$4}' > "$out")
 	if [ $? -eq 0 ]; then
 		do_slabs_plotting "$out"
 	fi

 	let lines=3
-	out=`basename "$in"`"-slabs-by-size"
-	`cat "$in" | grep -A "$lines" 'Slabs sorted by size' |\
+	out=$(basename "$in")"-slabs-by-size"
+	$(cat "$in" | grep -A "$lines" 'Slabs sorted by size' |\
 		grep -E -iv '\-\-|Name|Slabs'\
-		| awk '{print $1" "$4" "$4-$2*$3}' > "$out"`
+		| awk '{print $1" "$4" "$4-$2*$3}' > "$out")
 	if [ $? -eq 0 ]; then
 		do_slabs_plotting "$out"
 	fi

-	out=`basename "$in"`"-totals"
-	`cat "$in" | grep "Memory used" |\
-		awk '{print $3" "$7}' > "$out"`
+	out=$(basename "$in")"-totals"
+	$(cat "$in" | grep "Memory used" |\
+		awk '{print $3" "$7}' > "$out")
 	if [ $? -eq 0 ]; then
 		t_files[0]=$out
 		do_totals_plotting
--
2.49.0
Re: [PATCH V2] tools: mm: Added modern version of shell quote and a stanza
Posted by Pedro Falcato 3 months, 1 week ago
On Wed, Jul 02, 2025 at 12:19:18PM +0530, Bhaskar Chowdhury wrote:
> Three things precisely:
> 
> Replaced backquote with dollar-parentheses ...that is modern way of quoting in
> shell script. Improved readability.
> 
> Added a stranza of missing/required command for the operation,in this case
> gnuplot and that too in shell path.
> 
> And lastly,use "command -v" to search the command i.e gnuplot and the REASON
> for that :
> 
> The -v option tell show shell will invoke the command specified as its
> options.Basically to avoid dependency on something outside of the shell.It is
> also execute command found in the PATH. Essentially, ignoring other similar
> name stuff curated somewhere else in the system.
>

I don't understand what the point of this is, since we're not looking for the
executable, we're merely executing it. If command -v searches through PATH, it
will have the exact same result as just doing 'gnuplot'.

> Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
> ---
>  V2: What changes from V1: Verbose changelog, especially using command -v to
>  find the specific program i.e. gnuplot.
> 
>  tools/mm/slabinfo-gnuplot.sh | 44 ++++++++++++++++++++++++------------
>  1 file changed, 30 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/mm/slabinfo-gnuplot.sh b/tools/mm/slabinfo-gnuplot.sh
> index 873a892147e5..6c9add4bb8ad 100644
> --- a/tools/mm/slabinfo-gnuplot.sh
> +++ b/tools/mm/slabinfo-gnuplot.sh
> @@ -46,6 +46,22 @@ check_file_exist()
>  	fi
>  }
> 
> +# This variable could have space separated value
> +my_needed_commands="gnuplot"
> +
> +missing_counter=0
> +for needed_command in $my_needed_commands; do
> +  if ! hash "$needed_command" >/dev/null 2>&1; then
> +    printf "Command not found in PATH: %s\n" "$needed_command" >&2
> +    ((missing_counter++))
> +  fi
> +done
> +
> +if ((missing_counter > 0)); then
> +  printf "Minimum %d commands are missing in PATH, aborting\n" "$missing_counter" >&2
> +  exit 1
> +fi

And this seems unneeded. The script is quite minimal and simple, and it's also
named "gnuplot". Not sure what else you need there.

> +
>  do_slabs_plotting()
>  {
>  	local file=$1
> @@ -58,13 +74,13 @@ do_slabs_plotting()
> 
>  	check_file_exist "$file"
> 
> -	out_file=`basename "$file"`
> +	out_file=$(basename "$file")

But this sort of changes are probably welcome, though.
-- 
Pedro