diff mbox series

[libgpiod,2/8] tools: tests: don't declare and assign separately

Message ID 20240531134512.443850-3-warthog618@gmail.com
State New
Headers show
Series tools: tests: fix shellcheck warnings | expand

Commit Message

Kent Gibson May 31, 2024, 1:45 p.m. UTC
Fix shellcheck SC2155[1] - declare and assign separately to avoid
masking return values.

Signed-off-by: Kent Gibson <warthog618@gmail.com>

[1] https://www.shellcheck.net/wiki/SC2155
---
 tools/gpio-tools-test.bash | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

Comments

Bartosz Golaszewski June 3, 2024, 8:52 a.m. UTC | #1
On Fri, May 31, 2024 at 3:45 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> Fix shellcheck SC2155[1] - declare and assign separately to avoid
> masking return values.
>
> Signed-off-by: Kent Gibson <warthog618@gmail.com>
>
> [1] https://www.shellcheck.net/wiki/SC2155
> ---

Shouldn't the commit message say "*do* declare and assign separately"?

Bart

>  tools/gpio-tools-test.bash | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/tools/gpio-tools-test.bash b/tools/gpio-tools-test.bash
> index 34ea744..6bbb06c 100755
> --- a/tools/gpio-tools-test.bash
> +++ b/tools/gpio-tools-test.bash
> @@ -49,7 +49,8 @@ output_is() {
>
>  num_lines_is() {
>         [ "$1" -eq "0" ] || [ -z "$output" ] && return 0
> -       local NUM_LINES=$(echo "$output" | wc -l)
> +       local NUM_LINES
> +       NUM_LINES=$(echo "$output" | wc -l)
>         assertEquals " number of lines:" "$1" "$NUM_LINES"
>  }
>
> @@ -73,16 +74,18 @@ gpiosim_chip() {
>
>         for ARG in "$@"
>         do
> -               local KEY=$(echo $ARG | cut -d"=" -f1)
> -               local VAL=$(echo $ARG | cut -d"=" -f2)
> +               local KEY VAL
> +               KEY=$(echo "$ARG" | cut -d"=" -f1)
> +               VAL=$(echo "$ARG" | cut -d"=" -f2)
>
>                 if [ "$KEY" = "num_lines" ]
>                 then
>                         echo $VAL > $BANKPATH/num_lines
>                 elif [ "$KEY" = "line_name" ]
>                 then
> -                       local OFFSET=$(echo $VAL | cut -d":" -f1)
> -                       local LINENAME=$(echo $VAL | cut -d":" -f2)
> +                       local OFFSET LINENAME
> +                       OFFSET=$(echo "$VAL" | cut -d":" -f1)
> +                       LINENAME=$(echo "$VAL" | cut -d":" -f2)
>                         local LINEPATH=$BANKPATH/line$OFFSET
>
>                         mkdir -p $LINEPATH
> @@ -92,10 +95,11 @@ gpiosim_chip() {
>
>         echo 1 > $DEVPATH/live
>
> -       local chip_name=$(<$BANKPATH/chip_name)
> -       GPIOSIM_CHIP_NAME[$1]=$chip_name
> -       GPIOSIM_CHIP_PATH[$1]="/dev/$chip_name"
> -       GPIOSIM_DEV_NAME[$1]=$(<$DEVPATH/dev_name)
> +       local CHIP_NAME
> +       CHIP_NAME=$(<"$BANKPATH/chip_name")
> +       GPIOSIM_CHIP_NAME[$1]=$CHIP_NAME
> +       GPIOSIM_CHIP_PATH[$1]="/dev/$CHIP_NAME"
> +       GPIOSIM_DEV_NAME[$1]=$(<"$DEVPATH/dev_name")
>  }
>
>  gpiosim_chip_number() {
> @@ -3044,14 +3048,16 @@ die() {
>  # Must be done after we sources shunit2 as we need SHUNIT_VERSION to be set.
>  oneTimeSetUp() {
>         test "$SHUNIT_VERSION" = "$MIN_SHUNIT_VERSION" && return 0
> -       local FIRST=$(printf "$SHUNIT_VERSION\n$MIN_SHUNIT_VERSION\n" | sort -Vr | head -1)
> +       local FIRST
> +       FIRST=$(printf "$SHUNIT_VERSION\n$MIN_SHUNIT_VERSION\n" | sort -Vr | head -1)
>         test "$FIRST" = "$MIN_SHUNIT_VERSION" && \
>                 die "minimum shunit version required is $MIN_SHUNIT_VERSION (current version is $SHUNIT_VERSION"
>  }
>
>  check_kernel() {
>         local REQUIRED=$1
> -       local CURRENT=$(uname -r)
> +       local CURRENT
> +       CURRENT=$(uname -r)
>
>         SORTED=$(printf "$REQUIRED\n$CURRENT" | sort -V | head -n 1)
>
> --
> 2.39.2
>
Kent Gibson June 3, 2024, 8:58 a.m. UTC | #2
On Mon, Jun 03, 2024 at 10:52:39AM +0200, Bartosz Golaszewski wrote:
> On Fri, May 31, 2024 at 3:45 PM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > Fix shellcheck SC2155[1] - declare and assign separately to avoid
> > masking return values.
> >
> > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> >
> > [1] https://www.shellcheck.net/wiki/SC2155
> > ---
>
> Shouldn't the commit message say "*do* declare and assign separately"?
>

That is wrong, my bad.

I think I was going to change it to "don't declare and assign together"
and got distracted mid-edit.
So either that or simply "declare and assign separately".
A leading "do" would be redundant.

You ok to fix that, or would you like a re-spin?

Cheers,
Kent.
Bartosz Golaszewski June 3, 2024, 10:38 a.m. UTC | #3
On Mon, Jun 3, 2024 at 10:58 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Mon, Jun 03, 2024 at 10:52:39AM +0200, Bartosz Golaszewski wrote:
> > On Fri, May 31, 2024 at 3:45 PM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > Fix shellcheck SC2155[1] - declare and assign separately to avoid
> > > masking return values.
> > >
> > > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > >
> > > [1] https://www.shellcheck.net/wiki/SC2155
> > > ---
> >
> > Shouldn't the commit message say "*do* declare and assign separately"?
> >
>
> That is wrong, my bad.
>
> I think I was going to change it to "don't declare and assign together"
> and got distracted mid-edit.
> So either that or simply "declare and assign separately".
> A leading "do" would be redundant.
>
> You ok to fix that, or would you like a re-spin?
>

I'll fix it, others look good.

Bart
Kent Gibson June 3, 2024, 10:39 a.m. UTC | #4
On Mon, Jun 03, 2024 at 12:38:09PM +0200, Bartosz Golaszewski wrote:
> On Mon, Jun 3, 2024 at 10:58 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Mon, Jun 03, 2024 at 10:52:39AM +0200, Bartosz Golaszewski wrote:
> > > On Fri, May 31, 2024 at 3:45 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > > > Fix shellcheck SC2155[1] - declare and assign separately to avoid
> > > > masking return values.
> > > >
> > > > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > > >
> > > > [1] https://www.shellcheck.net/wiki/SC2155
> > > > ---
> > >
> > > Shouldn't the commit message say "*do* declare and assign separately"?
> > >
> >
> > That is wrong, my bad.
> >
> > I think I was going to change it to "don't declare and assign together"
> > and got distracted mid-edit.
> > So either that or simply "declare and assign separately".
> > A leading "do" would be redundant.
> >
> > You ok to fix that, or would you like a re-spin?
> >
>
> I'll fix it, others look good.
>

Thanks.

Kent.
diff mbox series

Patch

diff --git a/tools/gpio-tools-test.bash b/tools/gpio-tools-test.bash
index 34ea744..6bbb06c 100755
--- a/tools/gpio-tools-test.bash
+++ b/tools/gpio-tools-test.bash
@@ -49,7 +49,8 @@  output_is() {
 
 num_lines_is() {
 	[ "$1" -eq "0" ] || [ -z "$output" ] && return 0
-	local NUM_LINES=$(echo "$output" | wc -l)
+	local NUM_LINES
+	NUM_LINES=$(echo "$output" | wc -l)
 	assertEquals " number of lines:" "$1" "$NUM_LINES"
 }
 
@@ -73,16 +74,18 @@  gpiosim_chip() {
 
 	for ARG in "$@"
 	do
-		local KEY=$(echo $ARG | cut -d"=" -f1)
-		local VAL=$(echo $ARG | cut -d"=" -f2)
+		local KEY VAL
+		KEY=$(echo "$ARG" | cut -d"=" -f1)
+		VAL=$(echo "$ARG" | cut -d"=" -f2)
 
 		if [ "$KEY" = "num_lines" ]
 		then
 			echo $VAL > $BANKPATH/num_lines
 		elif [ "$KEY" = "line_name" ]
 		then
-			local OFFSET=$(echo $VAL | cut -d":" -f1)
-			local LINENAME=$(echo $VAL | cut -d":" -f2)
+			local OFFSET LINENAME
+			OFFSET=$(echo "$VAL" | cut -d":" -f1)
+			LINENAME=$(echo "$VAL" | cut -d":" -f2)
 			local LINEPATH=$BANKPATH/line$OFFSET
 
 			mkdir -p $LINEPATH
@@ -92,10 +95,11 @@  gpiosim_chip() {
 
 	echo 1 > $DEVPATH/live
 
-	local chip_name=$(<$BANKPATH/chip_name)
-	GPIOSIM_CHIP_NAME[$1]=$chip_name
-	GPIOSIM_CHIP_PATH[$1]="/dev/$chip_name"
-	GPIOSIM_DEV_NAME[$1]=$(<$DEVPATH/dev_name)
+	local CHIP_NAME
+	CHIP_NAME=$(<"$BANKPATH/chip_name")
+	GPIOSIM_CHIP_NAME[$1]=$CHIP_NAME
+	GPIOSIM_CHIP_PATH[$1]="/dev/$CHIP_NAME"
+	GPIOSIM_DEV_NAME[$1]=$(<"$DEVPATH/dev_name")
 }
 
 gpiosim_chip_number() {
@@ -3044,14 +3048,16 @@  die() {
 # Must be done after we sources shunit2 as we need SHUNIT_VERSION to be set.
 oneTimeSetUp() {
 	test "$SHUNIT_VERSION" = "$MIN_SHUNIT_VERSION" && return 0
-	local FIRST=$(printf "$SHUNIT_VERSION\n$MIN_SHUNIT_VERSION\n" | sort -Vr | head -1)
+	local FIRST
+	FIRST=$(printf "$SHUNIT_VERSION\n$MIN_SHUNIT_VERSION\n" | sort -Vr | head -1)
 	test "$FIRST" = "$MIN_SHUNIT_VERSION" && \
 		die "minimum shunit version required is $MIN_SHUNIT_VERSION (current version is $SHUNIT_VERSION"
 }
 
 check_kernel() {
 	local REQUIRED=$1
-	local CURRENT=$(uname -r)
+	local CURRENT
+	CURRENT=$(uname -r)
 
 	SORTED=$(printf "$REQUIRED\n$CURRENT" | sort -V | head -n 1)