dim: shellcheck fixes

PATCH In review Jani Nikula 2 participants 13 messages Mar 17, 2017 – Mar 20, 2017 Kbuild

Share & tools

Jani Nikula

author

2017-03-17 10:43 UTC #0 lore raw

cover dim: shellcheck fixes

Noticing that 64a54c5e1a5d ("dim: Add add-link command") added a condition that is always true (if [ -n $foo ]), despite my review of the patch, I refreshed some of my old patches to fix issues that can be caught by shellcheck.

Starting from patch 1, the minimal merge criteria for any new dim patches is that 'make shellcheck' passes.

I add shellcheck exceptions for starters, so there are no errors reported, and then remove the exceptions by fixing them one by one. I'm not sure if we'll ever fix them all, but let's at least make sure we're not adding any new silly mistakes.

BR,
Jani.

Jani Nikula (10):
dim: add make target to shellcheck dim
dim: quote check for non-zero string
dim: replace expr with $((..))
dim: use $* instead of $@ within string
dim: double quote array expansions
dim: quote expressions using { }
dim: add double quotes to prevent glob interpretation
dim: prevent * from becoming options
dim: avoid useless cat in apply-branch
dim: Replace .. with $(..)

 Makefile |  16 +++++++
 dim      | 143 ++++++++++++++++++++++++++++++++-------------------------------
 2 files changed, 88 insertions(+), 71 deletions(-)

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

author

2017-03-17 10:43 UTC #1 lore raw

PATCH 1/10 dim: add make target to shellcheck dim

Use 'make shellcheck' to check dim. Add a number of excludes to pass everything for starters. The goal is to fix issues that are worth fixing gradually, and making 'make shellcheck' merge criteria.

Signed-off-by Jani Nikula <[email protected]>

---
 Makefile | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Makefile +24 -0

index b43ae9c6fab3..96aed8581f9f 100644
@@ -19,6 +19,30 @@ drm-intel.html: drm-intel.rst drm-intel-flow.svg drm-intel-timeline.rst drm-inte
 
 dim.html: dim.rst
 
+SC_EXCLUDE := \
+	-e SC1083 \
+	-e SC2001 \
+	-e SC2002 \
+	-e SC2003 \
+	-e SC2005 \
+	-e SC2006 \
+	-e SC2034 \
+	-e SC2035 \
+	-e SC2046 \
+	-e SC2053 \
+	-e SC2068 \
+	-e SC2070 \
+	-e SC2086 \
+	-e SC2089 \
+	-e SC2090 \
+	-e SC2119 \
+	-e SC2120 \
+	-e SC2126 \
+	-e SC2145
+
+shellcheck:
+	shellcheck $(SC_EXCLUDE) dim
+
 clean:
 	rm -f drm-intel.html drm-intel-flow.svg dim.html drm-misc.html

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

author

2017-03-17 10:43 UTC #2 lore raw

PATCH 2/10 dim: quote check for non-zero string

In reply to Jani Nikula

Fix shellcheck SC2070: Always true because you failed to quote. Use [[ ]] instead.

Signed-off-by Jani Nikula <[email protected]>

---
 Makefile | 1 -
 dim      | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

Makefile +0 -1

index 96aed8581f9f..a3a1c8dba3ed 100644
@@ -31,7 +31,6 @@ SC_EXCLUDE := \
 	-e SC2046 \
 	-e SC2053 \
 	-e SC2068 \
-	-e SC2070 \
 	-e SC2086 \
 	-e SC2089 \
 	-e SC2090 \

dim +1 -1

index 64edcda83084..964dfcb655cb 100755
@@ -704,7 +704,7 @@ function dim_add_link
 
 	rm -f $file
 
-	if [ -n $message_id ]; then
+	if [[ -n "$message_id" ]]; then
 		dim_commit_add_tag "Link: http://patchwork.freedesktop.org/patch/msgid/$message_id"
 	else
 		echoerr "No message-id found in the patch file."

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

author

2017-03-17 10:43 UTC #3 lore raw

PATCH 3/10 dim: replace expr with $((..))

In reply to Jani Nikula

Fix shellcheck SC2003: expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]].

Signed-off-by Jani Nikula <[email protected]>

---
 Makefile | 1 -
 dim      | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

Makefile +0 -1

index a3a1c8dba3ed..a5cfd7b00d4b 100644
@@ -23,7 +23,6 @@ SC_EXCLUDE := \
 	-e SC1083 \
 	-e SC2001 \
 	-e SC2002 \
-	-e SC2003 \
 	-e SC2005 \
 	-e SC2006 \
 	-e SC2034 \

dim +1 -1

index 964dfcb655cb..26e8bffe011e 100755
@@ -157,7 +157,7 @@ while getopts hdfi opt; do
 			exit
 	esac
 done
-shift `expr $OPTIND - 1`
+shift $((OPTIND - 1))
 
 # first positional argument is the subcommand
 if [ -n "$HELP" -o "$#" = "0" ]; then

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

author

2017-03-17 10:43 UTC #4 lore raw

PATCH 4/10 dim: use $* instead of $@ within string

In reply to Jani Nikula

Fix shellcheck SC2145: Argument mixes string and array. Use * or separate argument.

Signed-off-by Jani Nikula <[email protected]>

---
 Makefile | 3 +--
 dim      | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

Makefile +1 -2

index a5cfd7b00d4b..d93cf2023949 100644
@@ -35,8 +35,7 @@ SC_EXCLUDE := \
 	-e SC2090 \
 	-e SC2119 \
 	-e SC2120 \
-	-e SC2126 \
-	-e SC2145
+	-e SC2126
 
 shellcheck:
 	shellcheck $(SC_EXCLUDE) dim

dim +1 -1

index 26e8bffe011e..6bc3cb9d66b4 100755
@@ -124,7 +124,7 @@ HELP=
 
 function echoerr
 {
-	echo "$dim: $@" >&2
+	echo "$dim: $*" >&2
 }
 
 function warn_or_fail

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

author

2017-03-17 10:43 UTC #5 lore raw

PATCH 5/10 dim: double quote array expansions

In reply to Jani Nikula

Fix shellcheck SC2068: Double quote array expansions, otherwise they're like $* and break on spaces.

Signed-off-by Jani Nikula <[email protected]>

---
 Makefile | 1 -
 dim      | 8 ++++----
 2 files changed, 4 insertions(+), 5 deletions(-)

Makefile +0 -1

index d93cf2023949..cf46a225d146 100644
@@ -29,7 +29,6 @@ SC_EXCLUDE := \
 	-e SC2035 \
 	-e SC2046 \
 	-e SC2053 \
-	-e SC2068 \
 	-e SC2086 \
 	-e SC2089 \
 	-e SC2090 \

dim +4 -4

index 6bc3cb9d66b4..2263620a2d7d 100755
@@ -1216,7 +1216,7 @@ function prep_pull_mail_overview
 	if [ "$#" = "0" ]; then
 		echo "*** insert pull request overview here ***"
 	else
-		for tag in $@ ; do
+		for tag in "$@"; do
 			obj=`git rev-parse $tag`
 			if [[ `git cat-file -t $obj` == "tag" ]] ; then
 				echo $tag:

@@ -1231,7 +1231,7 @@ function prep_pull_mail_overview
 function prep_pull_mail
 {
 	prep_pull_mail_greetings > ~/tmp/dim-pull-request
-	prep_pull_mail_overview $@ >> ~/tmp/dim-pull-request
+	prep_pull_mail_overview "$@" >> ~/tmp/dim-pull-request
 	prep_pull_mail_signature >> ~/tmp/dim-pull-request
 }

@@ -1273,11 +1273,11 @@ dim_alias_fw=for-each-workdirs
 function dim_for_each_workdirs
 {
 	cd $DIM_PREFIX/$DIM_DRM_INTEL
-	$@
+	"$@"
 	for branch in $dim_branches ; do
 		if [[ -d $DIM_PREFIX/$branch ]] ; then
 			cd $DIM_PREFIX/$branch
-			$@
+			"$@"
 		fi
 	done
 }

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

author

2017-03-17 10:43 UTC #6 lore raw

PATCH 6/10 dim: quote expressions using { }

In reply to Jani Nikula

Fix shellcheck SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.

Signed-off-by Jani Nikula <[email protected]>

---
 Makefile |  1 -
 dim      | 15 ++++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

Makefile +0 -1

index cf46a225d146..94ec71126223 100644
@@ -20,7 +20,6 @@ drm-intel.html: drm-intel.rst drm-intel-flow.svg drm-intel-timeline.rst drm-inte
 dim.html: dim.rst
 
 SC_EXCLUDE := \
-	-e SC1083 \
 	-e SC2001 \
 	-e SC2002 \
 	-e SC2005 \

dim +8 -7

index 2263620a2d7d..cff5135cd0de 100755
@@ -237,9 +237,10 @@ function url_to_remote # url
 
 function branch_to_remote # branch
 {
-	local remote
+	local branch remote
 
-	remote=`git rev-parse --abbrev-ref --symbolic-full-name $1@{upstream}`
+	branch=$1
+	remote=$(git rev-parse --abbrev-ref --symbolic-full-name "$branch@{upstream}")
 	remote=${remote%%/*}
 
 	echo $remote

@@ -280,7 +281,7 @@ function dim_uptodate
 		exit 1
 	fi
 
-	if ! git --git-dir=$DIM_PREFIX/maintainer-tools/.git show @{upstream}:dim |\
+	if ! git --git-dir=$DIM_PREFIX/maintainer-tools/.git show "@{upstream}:dim" |\
 			diff "$using" - >& /dev/null; then
 		echoerr "not running upstream version of the script."
 		exit 1

@@ -1365,7 +1366,7 @@ function dim_tag_next
 	cd $DIM_PREFIX/$DIM_DRM_INTEL
 	git fetch $DIM_DRM_INTEL_REMOTE
 
-	if [ $(git rev-parse drm-intel-next) == $(git rev-parse drm-intel-next@{u}) ] ; then
+	if [ $(git rev-parse drm-intel-next) == $(git rev-parse "drm-intel-next@{u}") ] ; then
 		echo "Tagging current drm-intel-next"
 
 		tag=drm-intel-next-$today

@@ -1403,14 +1404,14 @@ function dim_pull_request
 
 	if [ "$branch" = "drm-intel-next" ]; then
 		# drm-intel-next pulls have been tagged using dim update-next
-		drm_intel_next_tags=`git log $branch@{upstream} ^$upstream --decorate | grep "(.*tag: drm-intel-next-" | sed -e "s/^.*(.*tag: \(drm-intel-next-[^ ,]*\).*)$/\1/"`
+		drm_intel_next_tags=$(git log "$branch@{upstream}" ^$upstream --decorate | grep "(.*tag: drm-intel-next-" | sed -e "s/^.*(.*tag: \(drm-intel-next-[^ ,]*\).*)$/\1/")
 		prep_pull_mail $drm_intel_next_tags
-		tag=`git describe --all --exact $branch@{upstream}`
+		tag=$(git describe --all --exact "$branch@{upstream}")
 
 		repo="drm-intel"
 	else
 		tag=$branch-$today
-		$DRY git tag -f -a $tag $branch@{upstream}
+		$DRY git tag -f -a $tag "$branch@{upstream}"
 		$DRY git push -f $remote $tag
 		prep_pull_mail $tag

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

author

2017-03-17 10:43 UTC #7 lore raw

PATCH 7/10 dim: add double quotes to prevent glob interpretation

In reply to Jani Nikula

Fix shellcheck SC2053: Quote the rhs of = in [[ ]] to prevent glob interpretation.

Signed-off-by Jani Nikula <[email protected]>

---
 Makefile | 1 -
 dim      | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

Makefile +0 -1

index 94ec71126223..e6fa119d8f44 100644
@@ -27,7 +27,6 @@ SC_EXCLUDE := \
 	-e SC2034 \
 	-e SC2035 \
 	-e SC2046 \
-	-e SC2053 \
 	-e SC2086 \
 	-e SC2089 \
 	-e SC2090 \

dim +1 -1

index cff5135cd0de..2cf0a769bf47 100755
@@ -257,7 +257,7 @@ function branch_to_repo # branch
 		local repo branch override
 		read repo branch override <<< $conf
 
-		if [[ $branch == $1 ]] ; then
+		if [[ "$branch" == "$1" ]] ; then
 			echo $repo
 		fi
 	done

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

author

2017-03-17 10:43 UTC #8 lore raw

PATCH 8/10 dim: prevent * from becoming options

In reply to Jani Nikula

Fix shellcheck SC2035: Use ./*.patch so names with dashes won't become options.

Signed-off-by Jani Nikula <[email protected]>

---
 Makefile | 1 -
 dim      | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

Makefile +0 -1

index e6fa119d8f44..51da0dcc9dd3 100644
@@ -25,7 +25,6 @@ SC_EXCLUDE := \
 	-e SC2005 \
 	-e SC2006 \
 	-e SC2034 \
-	-e SC2035 \
 	-e SC2046 \
 	-e SC2086 \
 	-e SC2089 \

dim +1 -1

index 2cf0a769bf47..0eac9f627932 100755
@@ -588,7 +588,7 @@ function dim_rebuild_tip
 		git pull >& /dev/null
 		rm `rr_cache_dir`/rr-cache -Rf &> /dev/null || true
 		cp `rr_cache_dir`/* rr-cache -r
-		git add *.patch >& /dev/null || true
+		git add ./*.patch >& /dev/null || true
 		git add rr-cache/* > /dev/null
 		git rm rr-cache/rr-cache &> /dev/null || true
 		if git commit -m "$time: $integration_branch rerere cache update" >& /dev/null; then

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

author

2017-03-17 10:43 UTC #9 lore raw

PATCH 9/10 dim: avoid useless cat in apply-branch

In reply to Jani Nikula

Fix shellcheck SC2002: Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.

Signed-off-by Jani Nikula <[email protected]>

---
 Makefile | 1 -
 dim      | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

Makefile +0 -1

index 51da0dcc9dd3..ec04e75b41ec 100644
@@ -21,7 +21,6 @@ dim.html: dim.rst
 
 SC_EXCLUDE := \
 	-e SC2001 \
-	-e SC2002 \
 	-e SC2005 \
 	-e SC2006 \
 	-e SC2034 \

dim +2 -2

index 0eac9f627932..6dbfbed5e8f7 100755
@@ -675,7 +675,7 @@ function dim_apply_branch
 		sob=-s
 	fi
 
-	cat $file | git am --scissors -3 $sob "$@"
+	git am --scissors -3 $sob "$@" $file
 
 	if [ -n "$message_id" ]; then
 		dim_commit_add_tag "Link: http://patchwork.freedesktop.org/patch/msgid/$message_id"

@@ -887,7 +887,7 @@ dim_alias_mrr=magic-rebase-resolve
 function dim_magic_rebase_resolve
 {
 	git diff HEAD | patch -p1 -R
-	cat .git/rebase-merge/patch | dim_magic_patch
+	dim_magic_patch < .git/rebase-merge/patch
 	make $DIM_MAKE_OPTIONS
 	git add -u
 	git rebase --continue

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

author

2017-03-17 10:43 UTC #10 lore raw

PATCH 10/10 dim: Replace `..` with $(..)

In reply to Jani Nikula

Fix shellcheck SC2006: Use $(..) instead of deprecated ...

Unfortunately, uncovers one instance of shellcheck SC2143: Instead of [ -n $(foo | grep bar) ], use foo | grep -q bar. We'll accept that for now.

Signed-off-by Jani Nikula <[email protected]>

---
 Makefile |   4 +--
 dim      | 106 +++++++++++++++++++++++++++++++--------------------------------
 2 files changed, 55 insertions(+), 55 deletions(-)

Makefile +2 -2

index ec04e75b41ec..89b381c1e0aa 100644
@@ -22,7 +22,6 @@ dim.html: dim.rst
 SC_EXCLUDE := \
 	-e SC2001 \
 	-e SC2005 \
-	-e SC2006 \
 	-e SC2034 \
 	-e SC2046 \
 	-e SC2086 \

@@ -30,7 +29,8 @@ SC_EXCLUDE := \
 	-e SC2090 \
 	-e SC2119 \
 	-e SC2120 \
-	-e SC2126
+	-e SC2126 \
+	-e SC2143
 
 shellcheck:
 	shellcheck $(SC_EXCLUDE) dim

dim +53 ~~-53~~

350 lines not on this page · raw message

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Daniel Vetter

2017-03-17 16:53 UTC #11 lore raw

In reply to Jani Nikula

Quoted 5 lines · Jani Nikula

Noticing that 64a54c5e1a5d ("dim: Add add-link command") added a condition that is always true (if [ -n $foo ]), despite my review of the patch, I refreshed some of my old patches to fix issues that can be caught by shellcheck.

Quoted 7 lines

Starting from patch 1, the minimal merge criteria for any new dim patches is that 'make shellcheck' passes.
I add shellcheck exceptions for starters, so there are no errors reported, and then remove the exceptions by fixing them one by one. I'm not sure if we'll ever fix them all, but let's at least make sure we're not adding any new silly mistakes.

Ack on the entire series. checkpatch for dim!
-Daniel

Quoted 25 lines

BR,
Jani.
Jani Nikula (10):
dim: add make target to shellcheck dim
dim: quote check for non-zero string
dim: replace expr with $((..))
dim: use $* instead of $@ within string
dim: double quote array expansions
dim: quote expressions using { }
dim: add double quotes to prevent glob interpretation
dim: prevent * from becoming options
dim: avoid useless cat in apply-branch
dim: Replace .. with $(..)
 Makefile |  16 +++++++
 dim      | 143 ++++++++++++++++++++++++++++++++-------------------------------
 2 files changed, 88 insertions(+), 71 deletions(-)
Signature · 1 line
2.1.4

Signature · 7 lines

Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula

author

2017-03-20 13:14 UTC #12 lore raw

Quoted 9 lines · Daniel Vetter

Quoted 8 lines · Jani Nikula
Noticing that 64a54c5e1a5d ("dim: Add add-link command") added a condition that is always true (if [ -n $foo ]), despite my review of the patch, I refreshed some of my old patches to fix issues that can be caught by shellcheck.
Starting from patch 1, the minimal merge criteria for any new dim patches is that 'make shellcheck' passes.

Quoted 6 lines

Quoted 4 lines
I add shellcheck exceptions for starters, so there are no errors reported, and then remove the exceptions by fixing them one by one. I'm not sure if we'll ever fix them all, but let's at least make sure we're not adding any new silly mistakes.
Ack on the entire series. checkpatch for dim!

Thanks, pushed.

BR,
Jani.

Signature · 5 lines

Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx