[PATCH] tools: mm: Added modern version of shell quote and a stranza for required command check

[PATCH] tools: mm: Added modern version of shell quote and a stranza for required command check

[Bhaskar Chowdhury]

Three things precisely:

Replaed backquote with dollar-parentheses ...that is modern way of quoting in
shell script.

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

And lastly, command with -v print the path of command i.e location for the
command to be executed.


Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
---
 tools/mm/slabinfo-gnuplot.sh | 45 +++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/tools/mm/slabinfo-gnuplot.sh b/tools/mm/slabinfo-gnuplot.sh
index 873a892147e5..de621963b3d1 100644
--- a/tools/mm/slabinfo-gnuplot.sh
+++ b/tools/mm/slabinfo-gnuplot.sh
@@ -38,6 +38,23 @@ usage()
 	echo "-r %d,%d		- use data samples from a given range"
 }

+
+# This varialble could have space separated commands
+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
+
 check_file_exist()
 {
 	if [ ! -f "$1" ]; then
@@ -58,13 +75,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 +95,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 +130,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 +165,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] tools: mm: Added modern version of shell quote and a stranza for required command check

[Christoph Berg]

> -gnuplot -p << EOF
> +$(command -v gnuplot) -p << EOF

Why would you delegate PATH searching to `command -v` when you can
just invoke the command directly?

Christoph

Re: [PATCH] tools: mm: Added modern version of shell quote and a stranza for required command check

[Andrew Morton]

On Tue, 1 Jul 2025 16:49:27 +0530 Bhaskar Chowdhury <unixbhaskar@gmail.com> wrote:

> On 12:57 Tue 01 Jul 2025, Christoph Berg wrote:
> >> -gnuplot -p << EOF
> >> +$(command -v gnuplot) -p << EOF
> >
> >Why would you delegate PATH searching to `command -v` when you can
> >just invoke the command directly?
> >
> 
> Because, for the two reasons,
> 
> One posix compliant behavior of the command
> 
> Second, the binary could be reside other places than the PATH. And this damn
> thing make sure, the binary in the path should execute only.

I still don't think I understand that :(

Can you please have another pass through the changelog, explain things
more fully and resend?

Thanks.


Also,

> +missing_counter=0
>
> ...
>
> +if ((missing_counter > 0)); then

I'd have used $missing_counter here, but what you have appears to work.
I don't know why :(


> +  printf "Minimum %d commands are missing in PATH, aborting\n" "$missing_counter" >&2
> +  exit 1
> +fi