[PATCH 13/22] tools/perf/tests: Fix shellcheck warning for coresight.sh shell script

James Clark james.clark at arm.com
Tue Jun 27 17:52:02 AEST 2023



On 21/06/2023 09:30, Athira Rajeev wrote:
> From: Kajol Jain <kjain at linux.ibm.com>
> 
> Running shellcheck on coresight.sh throws below warning:
> 
> In lib/coresight.sh line 1:
> ^-- SC2148 (error): Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
> 
> In lib/coresight.sh line 13:
> PERFRECOPT="$PERFRECMEM -e cs_etm//u"
> ^--------^ SC2034 (warning): PERFRECOPT appears unused. Verify use (or export if used externally).
> 
> Fixed the warnings by:
> - Adding shell directive
> - Using export for PERFRECOPT variable
> 
> Signed-off-by: Athira Rajeev <atrajeev at linux.vnet.ibm.com>
> Signed-off-by: Kajol Jain <kjain at linux.ibm.com>
> ---
>  tools/perf/tests/shell/lib/coresight.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/shell/lib/coresight.sh b/tools/perf/tests/shell/lib/coresight.sh
> index 6c3d34ec64d8..c7d97a0186c7 100644
> --- a/tools/perf/tests/shell/lib/coresight.sh
> +++ b/tools/perf/tests/shell/lib/coresight.sh
> @@ -1,3 +1,4 @@
> +#!/bin/bash

There is a comment a couple of lines below this that says the shebang
was excluded for a reason. It's probably worth leaving it excluded or
updating the comment.

>  # SPDX-License-Identifier: GPL-2.0
>  # Carsten Haitzler <carsten.haitzler at arm.com>, 2021
>  
> @@ -10,7 +11,7 @@
>  
>  # perf record options for the perf tests to use
>  PERFRECMEM="-m ,16M"
> -PERFRECOPT="$PERFRECMEM -e cs_etm//u"
> +export PERFRECOPT="$PERFRECMEM -e cs_etm//u"

This might silence the warning, but there are other variables like DATD
that are used in the sub-test scripts in the same way. They just don't
trigger the warning because they're also used in the top level one.

I think this change makes the code more misleading than solving any
problems. The coresight.sh script is sourced in the same process as the
sub tests so exporting variables isn't needed which is why it already
works without this change.

>  
>  TOOLS=$(dirname $0)
>  DIR="$TOOLS/$TEST"


More information about the Linuxppc-dev mailing list