[PATCH v2] [gdb/contrib] Make gdb-add-index.sh shellcheck-clean

[PATCH v2] [gdb/contrib] Make gdb-add-index.sh shellcheck-clean

[Tom de Vries]

Make gdb-add-index.sh shellcheck-clean:
- drop x in "xa" != "xb" comparison
- eliminate tmp_files
- drop dwz_file=$(echo $dwz_file) and strip leading spaces using
  sed instead

Also for clarity:
- use $file instead of $1
- use set -- after consuming last argument

Changes in v2:
- use "$file" instead of $1
- to eliminate tmp_files, don't use $@ but list tmp files explicitly in
  cleanup function

Versions:
- v1 https://sourceware.org/pipermail/gdb-patches/2026-September/230364.html
---
 gdb/contrib/gdb-add-index.sh | 38 +++++++++++++++++++++---------------
 gdb/contrib/shellcheck.sh    |  4 ----
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
index 4a30c5c5536..0c7797d23ed 100755
--- a/gdb/contrib/gdb-add-index.sh
+++ b/gdb/contrib/gdb-add-index.sh
@@ -82,7 +82,7 @@ case ${opt} in
 	;;
 esac
 # Break from loop if the first character of OPT is not '-'.
-[ "x$(printf %.1s "$opt")" != "x-" ]
+[ "$(printf %.1s "$opt")" != "-" ]
 do
     shift
 done
@@ -93,10 +93,11 @@ if test $# != 1; then
 fi
 
 file="$1"
+set --
 
 if test -L "$file"; then
     if ! command -v readlink >/dev/null 2>&1; then
-	echo "$myname: 'readlink' missing.  Failed to follow symlink $1." 1>&2
+	echo "$myname: 'readlink' missing.  Failed to follow symlink $file." 1>&2
 	exit 1
     fi
 
@@ -135,8 +136,7 @@ if $READELF -S "$file" | grep -q " \.gnu_debugaltlink "; then
     dwz_file=$($READELF --string-dump=.gnu_debugaltlink "$file" \
 		   | grep -A1  "'\.gnu_debugaltlink':" \
 		   | tail -n +2 \
-		   | sed 's/.*]//')
-    dwz_file=$(echo $dwz_file)
+		   | sed 's/.*] *//')
     if $READELF -S "$dwz_file" | grep -E -q " \.(gdb_index|debug_names) "; then
 	# Already has an index, skip it.
 	dwz_file=""
@@ -154,19 +154,25 @@ set_files ()
     debugstrerr="${fpath}.debug_str.err"
 }
 
-tmp_files=
-for f in "$file" "$dwz_file"; do
-    if [ "$f" = "" ]; then
-	continue
-    fi
-    set_files "$f"
-    tmp_files="$tmp_files $index4 $index5 $debugstr $debugstrmerge $debugstrerr"
-done
-
-rm -f $tmp_files
-
 # Ensure intermediate index file is removed when we exit.
-trap "rm -f $tmp_files" 0
+cleanup ()
+{
+    for f in "$file" "$dwz_file"; do
+	if [ "$f" = "" ]; then
+	    continue
+	fi
+	set_files "$f"
+	rm -f \
+	   "$index4" \
+	   "$index5" \
+	   "$debugstr" \
+	   "$debugstrmerge" \
+	   "$debugstrerr"
+    done
+}
+
+cleanup
+trap cleanup 0
 
 $GDB --batch -nx -iex 'set auto-load no' \
     -iex 'set debuginfod enabled off' \
diff --git a/gdb/contrib/shellcheck.sh b/gdb/contrib/shellcheck.sh
index 6417d7a871f..9056206ea5f 100755
--- a/gdb/contrib/shellcheck.sh
+++ b/gdb/contrib/shellcheck.sh
@@ -41,10 +41,6 @@ for f in "$@"; do
 	    # Skip generated files.
 	    continue
 	    ;;
-	gdb/contrib/gdb-add-index.sh )
-	    # Skip unclean files.
-	    continue
-	    ;;
 	*)
 	    files=("${files[@]}" "$f")
 	    ;;

base-commit: a121bf759aefc18f3be9c5495d23ebe39354810b
-- 
2.51.0

Re: [PATCH v2] [gdb/contrib] Make gdb-add-index.sh shellcheck-clean

[Keith Seitz]

Hi,

On 9/23/26 12:13 AM, Tom de Vries wrote:
> diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
> index 4a30c5c5536..0c7797d23ed 100755
> --- a/gdb/contrib/gdb-add-index.sh
> +++ b/gdb/contrib/gdb-add-index.sh
> @@ -93,10 +93,11 @@ if test $# != 1; then
>   fi
>   
>   file="$1"
> +set --

Does this "set --" accomplish anything anymore?

Otherwise, everything looks good. Thank you!

Reviewed-By: Keith Seitz <keiths@redhat.com>

Keith

Re: [PATCH v2] [gdb/contrib] Make gdb-add-index.sh shellcheck-clean

[Tom de Vries]

On 9/23/26 5:21 PM, Keith Seitz wrote:
> Hi,
> 
> On 9/23/26 12:13 AM, Tom de Vries wrote:
>> diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
>> index 4a30c5c5536..0c7797d23ed 100755
>> --- a/gdb/contrib/gdb-add-index.sh
>> +++ b/gdb/contrib/gdb-add-index.sh
>> @@ -93,10 +93,11 @@ if test $# != 1; then
>>   fi
>>   file="$1"
>> +set --
> 
> Does this "set --" accomplish anything anymore?
> 

Yes.  The idea was to drop the arguments once we don't need them 
anymore, to make sure subsequent $1 usage doesn't work anymore, and 
we're forced use $file instead.

I've added a comment about that:
...
  file="$1"
+
+# All arguments have been processed, there's no need to have them
+# around anymore.
  set --
...
and pushed.

> Otherwise, everything looks good. Thank you!
> 

Thanks for the reviews.

- Tom

> Reviewed-By: Keith Seitz <keiths@redhat.com>
> 
> Keith
>