improvements from using shellcheck

2 files changed, 56 insertions, 57 deletions

diff --git a/Makefile b/Makefile
index 295747e..4f93756 100644
--- a/Makefile
+++ b/Makefile
@@ -10,3 +10,7 @@ work/data/$(PREFIX):
 	mkdir -p "$@"
 	cp git-repo "$@"
 
+.PHONY: shellcheck
+shellcheck:
+	shellcheck --shell=bash --color=never git-repo
+
diff --git a/git-repo b/git-repo
index 6e910bc..ea047bf 100755
--- a/git-repo
+++ b/git-repo
@@ -1,31 +1,33 @@
 #!/usr/bin/env bash
 BASEDIR=/git
 
-C_bg_green=$(echo -e "\e[42m")
-C_bg_magenta=$(echo -e "\e[45m")
-C_bg_default=$(echo -e "\e[49m")
-C_bg_lblue=$(echo -e "\e[104m")
+C_bg_green=$(echo -e "\\e[42m")
+C_bg_magenta=$(echo -e "\\e[45m")
+C_bg_default=$(echo -e "\\e[49m")
+C_bg_lblue=$(echo -e "\\e[104m")
 
 _getent() {
     local db=$1
     local key=$2
-    getent $db $key | cut -d ':' -f 1
+    getent "$db" "$key" | cut -d ':' -f 1
 }
 
 _list_repo_dirs() {
     find "$1" -maxdepth 6 -type d -not -path '*.git/*' -name '*.git' 2>/dev/null
 }
 
+# shellcheck disable=SC2154
 do_list() {
+    local root_path
     if [ -n "$1" ]; then
-	local root_path=$(readlink -f "$1")
+	root_path=$(readlink -f "$1")
     else
-	local root_path=$BASEDIR
+	root_path=$BASEDIR
     fi
     echo -n "Working.. listing repositories under $root_path"
     for repo_path in $(_list_repo_dirs $root_path | sort); do
-        echo -ne "\r" # to remove working
-	eval $(stat --format 'local repo_path_uid=%u repo_path_gid=%g repo_path_mode=%a' "$repo_path")
+        echo -ne "\\r" # to remove "Working..."
+	eval "$(stat --format 'local repo_path_uid=%u repo_path_gid=%g repo_path_mode=%a' "$repo_path")"
         if [ "$repo_path_uid" != "$UID" ]; then
             # so there must be a matching gid
             local pass=false
@@ -40,11 +42,12 @@ do_list() {
 
         fi
 	
-        local shortdesc=$(cut -c 1-40 < "$repo_path/description")
-	local permissions="owner=$(_getent passwd $repo_path_uid)"
-	local color="$C_bg_green"
+        local shortdesc permissions color
+	shortdesc=$(cut -c 1-40 < "$repo_path/description")
+	permissions="owner=$(_getent passwd "$repo_path_uid")"
+	color="$C_bg_green"
         if [ $(( 0$repo_path_mode & 070 )) -gt 0 ]; then # shared
-	    permissions="$permissions group=$(_getent group $repo_path_gid)"
+	    permissions="$permissions group=$(_getent group "$repo_path_gid")"
 	    color="$C_bg_lblue"
 	fi
         if [ $(( 0$repo_path_mode & 07 )) -gt 0 ]; then # public
@@ -54,7 +57,7 @@ do_list() {
             fi
 	    color="$C_bg_magenta"
 	fi
-	printf "%s%-60s (%s) (%s)\n" "$color" "$repo_path$C_bg_default" "$permissions" "$shortdesc"
+	printf "%s%-60s (%s) (%s)\\n" "$color" "$repo_path$C_bg_default" "$permissions" "$shortdesc"
     done
 }
 
@@ -64,10 +67,10 @@ _create_repo() {
     local group=$3
     local is_public=$4
 
-    read -p 'New repository name.git: '
+    read -r -p 'New repository name.git: '
     local repo_name=$REPLY
 
-    if ! `expr "$repo_name" : '.*\.git' >/dev/null`; then
+    if ! expr "$repo_name" : '.*\.git' >/dev/null; then
 	echo "Repository name must end with .git"
 	return 1
     fi
@@ -76,14 +79,14 @@ _create_repo() {
 
     test \! -e "$repo_dir" || { echo "Repo $repo_dir already exist"; exit 1; }
 
-    read -p 'Set Description:         '
+    read -r -p 'Set Description:         '
     local repo_desc=$REPLY
 
-    read -p "Create $label in $repo_dir Ok? (y/N)"
+    read -r -p "Create $label in $repo_dir Ok? (y/N)"
     test "$REPLY" == "y" || exit 1
 
     mkdir -p "$repo_dir"
-    git init -q --bare --shared=$shared "$repo_dir"
+    git init -q --bare --shared="$shared" "$repo_dir"
     git config receive.denyNonFastforwards false
     chgrp -R "$group" "$repo_dir"
     echo "$repo_desc" > "$repo_dir/description"
@@ -114,7 +117,8 @@ do_create_private() {
 }
 
 do_show() {
-    for repo_path in $*; do
+    local repo_file_mode repo_file_uid repo_file_user repo_file_gid repo_file_group repo_git_shared
+    for repo_path in "$@"; do
 	if ! [ -f "$repo_path/HEAD" ]; then
 	    repo_path="$BASEDIR/$repo_path"
 	fi
@@ -122,13 +126,12 @@ do_show() {
             echo "Not a git repository: $repo_path"
             continue
         fi
-
-        local repo_file_mode=$(stat --format %a "$repo_path")
-        local repo_file_uid=$(stat --format %u "$repo_path")
-        local repo_file_user=$(_getent passwd $repo_file_uid)
-        local repo_file_gid=$(stat --format %g "$repo_path")
-        local repo_file_group=$(_getent group $repo_file_gid)
-        local repo_git_shared=$(GIT_DIR="$repo_path" git config --get core.sharedrepository)
+        repo_file_mode=$(stat --format %a "$repo_path")
+        repo_file_uid=$(stat --format %u "$repo_path")
+        repo_file_user=$(_getent passwd "$repo_file_uid")
+        repo_file_gid=$(stat --format %g "$repo_path")
+        repo_file_group=$(_getent group "$repo_file_gid")
+        repo_git_shared=$(GIT_DIR="$repo_path" git config --get core.sharedrepository)
 
 	echo "  Directory: $repo_path"
         echo "  ✔ Permissions mode ${repo_file_mode} (uid=${repo_file_uid}/${repo_file_user} gid=${repo_file_gid}/${repo_file_group})"
@@ -144,9 +147,10 @@ do_show() {
 }
 
 do_mirror() {
-    local root_path=$(readlink -f "$1")
-    for repo_path in $(_list_repo_dirs $root_path); do
-	local is_mirror=$(GIT_DIR="$repo_path" git config --get remote.origin.mirror)
+    local root_path is_mirror
+    root_path=$(readlink -f "$1")
+    for repo_path in $(_list_repo_dirs "$root_path"); do
+	is_mirror=$(GIT_DIR="$repo_path" git config --get remote.origin.mirror)
 	if [ "$is_mirror" != "true" ]; then
 	    continue # skip non-mirror repo
 	fi
@@ -155,42 +159,33 @@ do_mirror() {
     done
 }
 
-do_make_public() {
+_do_make() {
     local repo_path=$1
+    local repo_file_mode=$2
+    local repo_dir_mode=$3
+    local repo_is_public=$4
     test -f "$repo_path/HEAD" || { echo "$repo_path is not a git repository"; exit 1; }
     set -x
-    git init -q --bare --shared=0664 "$repo_path"
-    find "$repo_path" -type d -exec chown $USER:share \{\} \;
-    find "$repo_path" -type d -exec chmod 0775 \{\} \;
-    find "$repo_path" -type f -exec chmod 0664 \{\} \;
-    touch "$repo_path/PUBLIC"
+    git init -q --bare --shared="$repo_file_mode" "$repo_path"
+    find "$repo_path" -type d -exec chown "$USER:share" \{\} \;
+    find "$repo_path" -type d -exec chmod "$repo_dir_mode" \{\} \;
+    find "$repo_path" -type f -exec chmod "$repo_file_mode" \{\} \;
+    if $repo_is_public; then
+	touch "$repo_path/PUBLIC"
+    fi
     set +x
 }
 
+do_make_public() {
+    _do_make "$1" 0664 0775 true
+}
+
 do_make_shared() {
-    local repo_path=$1
-    test -f "$repo_path/HEAD" || { echo "$repo_path is not a git repository"; exit 1; }
-    set -x
-    git init -q --bare --shared=0660 "$repo_path"
-    find "$repo_path" -type d -exec chown $USER:share \{\} \;
-    find "$repo_path" -type d -exec chmod 0770 \{\} \;
-    find "$repo_path" -type f -exec chmod 0660 \{\} \;
-    chgrp -R share "$repo_path"
-    rm -f "$repo_path/PUBLIC"
-    set +x
+    _do_make "$1" 0660 0770 false
 }
 
 do_make_private() {
-    local repo_path=$1
-    test -f "$repo_path/HEAD" || { echo "$repo_path is not a git repository"; exit 1; }
-    set -x
-    git init -q --bare --shared=0600 "$repo_path"
-    find "$repo_path" -type d -exec chown $USER:$USER \{\} \;
-    find "$repo_path" -type d -exec chmod 0700 \{\} \;
-    find "$repo_path" -type f -exec chmod 0600 \{\} \;
-    chgrp -R $USER "$repo_path"
-    rm -f "$repo_path/PUBLIC"
-    set +x
+    _do_make "$1" 0600 0700 false
 }
 
 case "$1" in
@@ -203,7 +198,7 @@ case "$1" in
     make-private) do_make_private "$2" ;;
     show)
         shift
-        do_show $*
+        do_show "$@"
         ;;
     mirror) do_mirror "$2" ;;
     *)