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

[Pedro Falcato]

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