[PATCH v2 1/5] [gdb/testsuite] Clean up gdb/contrib/expect-read1.sh

[PATCH v2 1/5] [gdb/testsuite] Clean up gdb/contrib/expect-read1.sh

[Tom de Vries]

Clean up script gdb/contrib/expect-read1.sh by:
- fixing shellcheck warnings,
- using mktemp (which takes TMPDIR into account) instead of a hardcoded
  "/tmp/expect-read1.$$.so",
- adding comments, and
- adding emacs / vi settings for local tab size 2 (copied from ./ltmain.sh).

Tested on x86_64-linux.
---
 gdb/contrib/expect-read1.sh | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/gdb/contrib/expect-read1.sh b/gdb/contrib/expect-read1.sh
index 890a1cdbd07..82c638c59d2 100755
--- a/gdb/contrib/expect-read1.sh
+++ b/gdb/contrib/expect-read1.sh
@@ -26,17 +26,32 @@
 # or
 # bash$ EXPECT=../contrib/expect-read1.sh runtest
 
-C=`echo $0|sed 's/\.sh$/.c/'`
-if ! test -e $C; then
+# Find source file.
+C=$(echo "$0" | sed 's/\.sh$/.c/')
+if ! test -e "$C"; then
   echo >&2 "$0: Cannot find 'srcdir/gdb/contrib/expect-read1.c' at '$C'."
   exit 2
 fi
-SO=/tmp/expect-read1.$$.so
-rm -f $SO
+
+# Create temp directory.
+tmpdir=$(mktemp -d)
+
+# Schedule cleanup.
+trap 'rm -rf $tmpdir' EXIT
+
+# Compile shared library.
+SO=$tmpdir/expect-read1.so
 CMD="${CC_FOR_TARGET:-gcc} -o $SO -Wall -fPIC -shared $C"
 if ! $CMD; then
   echo >&2 "$0: Failed: $CMD"
   exit 2
 fi
-trap "rm -f $SO" EXIT
+
+# Call expect with the shared library preloaded.
 LD_PRELOAD=$SO expect "$@"
+
+# Local Variables:
+# mode:shell-script
+# sh-indentation:2
+# End:
+# vi:sw=2

base-commit: 888ff82e77d9ab8f04893a68cd6b4f518d6b50d9
-- 
2.35.3

[PATCH v2 2/5] [gdb/testsuite] Clean up gdb/contrib/cc-with-tweaks.sh

[Tom de Vries]

Fix shellcheck warnings in script gdb/contrib/cc-with-tweaks.sh.

Tested on x86_64-linux.
---
 gdb/contrib/cc-with-tweaks.sh | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/gdb/contrib/cc-with-tweaks.sh b/gdb/contrib/cc-with-tweaks.sh
index f760bd7c0a1..9d9c19c4430 100755
--- a/gdb/contrib/cc-with-tweaks.sh
+++ b/gdb/contrib/cc-with-tweaks.sh
@@ -50,7 +50,7 @@
 # If nothing is given, no changes are made
 
 myname=cc-with-tweaks.sh
-mydir=`dirname "$0"`
+mydir=$(dirname "$0")
 
 if [ -z "$GDB" ]
 then
@@ -112,7 +112,7 @@ if [ "$want_index" = true ]
 then
     if [ -z "$GDB_ADD_INDEX" ]
     then
-	if [ -f $mydir/gdb-add-index.sh ]
+	if [ -f "$mydir/gdb-add-index.sh" ]
 	then
 	    GDB_ADD_INDEX="$mydir/gdb-add-index.sh"
 	else
@@ -200,7 +200,7 @@ if [ "$want_index" = true ]; then
 	cp "$f" "$tmpdir"
     done
 
-    tmpfile="$tmpdir/$(basename $output_file)"
+    tmpfile=$tmpdir/$(basename "$output_file")
     # Filter out these messages which would stop dejagnu testcase run:
     # echo "$myname: No index was created for $file" 1>&2
     # echo "$myname: [Was there no debuginfo? Was there already an index?]" 1>&2
@@ -209,7 +209,7 @@ if [ "$want_index" = true ]; then
     rc=${PIPESTATUS[0]}
     mv "$tmpfile" "$output_file"
     rm -f "$tmpdir"/*.dwo
-    [ $rc != 0 ] && exit $rc
+    [ "$rc" != 0 ] && exit "$rc"
 fi
 
 if [ "$want_index_cache" = true ]; then
@@ -268,9 +268,9 @@ elif [ "$want_multi" = true ]; then
     # new file in case dwz fails.
     rm -f "$dwz_file"
 
-    cp $output_file ${output_file}.alt
-    $DWZ -m "$dwz_file" "$output_file" ${output_file}.alt > /dev/null
-    rm -f ${output_file}.alt
+    cp "$output_file" "${output_file}.alt"
+    $DWZ -m "$dwz_file" "$output_file" "${output_file}.alt" > /dev/null
+    rm -f "${output_file}.alt"
 
     # Validate dwz's work by checking if the expected output file exists.
     if [ ! -f "$dwz_file" ]; then
@@ -280,14 +280,19 @@ elif [ "$want_multi" = true ]; then
 fi
 
 if [ "$want_dwp" = true ]; then
-    dwo_files=$($READELF -wi "${output_file}" | grep _dwo_name | \
-	sed -e 's/^.*: //' | sort | uniq)
+    mapfile -t dwo_files \
+	    < \
+	    <($READELF -wi "${output_file}" \
+		  | grep _dwo_name \
+		  | sed -e 's/^.*: //' \
+		  | sort \
+		  | uniq)
     rc=0
-    if [ -n "$dwo_files" ]; then
-	$DWP -o "${output_file}.dwp" ${dwo_files} > /dev/null
+    if  [ ${#dwo_files[@]} -ne 0 ]; then
+	$DWP -o "${output_file}.dwp" "${dwo_files[@]}" > /dev/null
 	rc=$?
 	[ $rc != 0 ] && exit $rc
-	rm -f ${dwo_files}
+	rm -f "${dwo_files[@]}"
     fi
 fi
 
@@ -322,9 +327,9 @@ if [ "$want_gnu_debuglink" = true ]; then
 	# .gnu_debuglink to debug_file.
 	$OBJCOPY --add-gnu-debuglink="$link" "${stripped_file}" \
 		"${output_file}"
-	rc=$?
-	[ $rc != 0 ] && exit $rc
     )
+    rc=$?
+    [ $rc != 0 ] && exit $rc
 fi
 
 exit $rc
-- 
2.35.3

[PATCH v2 3/5] [gdb/testsuite] Clean up formatting in gdb/contrib/cc-with-tweaks.sh

[Tom de Vries]

In emacs, on gdb/contrib/cc-with-tweaks.sh, do:
- M-x whitespace-cleanup,
- M-x mark-whole-buffer and M-x indent-region, and
- and undo the unwanted changes in the header comment.

Only whitespace changes.

Tested on x86_64-linux.
---
 gdb/contrib/cc-with-tweaks.sh | 40 +++++++++++++++++------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/gdb/contrib/cc-with-tweaks.sh b/gdb/contrib/cc-with-tweaks.sh
index 9d9c19c4430..4214b929295 100755
--- a/gdb/contrib/cc-with-tweaks.sh
+++ b/gdb/contrib/cc-with-tweaks.sh
@@ -136,10 +136,10 @@ do
     # doing a link and what the output file is.
     # It's not perfect, but it seems to work well enough for the task at hand.
     case "$arg" in
-    "-c") have_link=no ;;
-    "-E") have_link=no ;;
-    "-S") have_link=no ;;
-    "-o") next_is_output_file=yes ;;
+	"-c") have_link=no ;;
+	"-E") have_link=no ;;
+	"-S") have_link=no ;;
+	"-o") next_is_output_file=yes ;;
     esac
 done
 
@@ -214,9 +214,9 @@ fi
 
 if [ "$want_index_cache" = true ]; then
     $GDB -q -batch \
-	-ex "set index-cache directory $INDEX_CACHE_DIR" \
-	-ex "set index-cache enabled on" \
-	-ex "file $output_file"
+	 -ex "set index-cache directory $INDEX_CACHE_DIR" \
+	 -ex "set index-cache enabled on" \
+	 -ex "file $output_file"
     rc=$?
     [ $rc != 0 ] && exit $rc
 fi
@@ -248,18 +248,18 @@ if [ "$want_dwz" = true ]; then
     rm -f "${output_file}.copy"
 
     case $cmp_rc in
-    0)
-	echo "$myname: dwz did not modify ${output_file}."
-        exit 1
-	;;
-    1)
-	# File was modified, great.
-	;;
-    *)
-	# Other cmp error, it presumably has already printed something on
-	# stderr.
-	exit 1
-	;;
+	0)
+	    echo "$myname: dwz did not modify ${output_file}."
+	    exit 1
+	    ;;
+	1)
+	    # File was modified, great.
+	    ;;
+	*)
+	    # Other cmp error, it presumably has already printed something on
+	    # stderr.
+	    exit 1
+	    ;;
     esac
 elif [ "$want_multi" = true ]; then
     get_tmpdir
@@ -326,7 +326,7 @@ if [ "$want_gnu_debuglink" = true ]; then
 	# Overwrite output_file with stripped version containing
 	# .gnu_debuglink to debug_file.
 	$OBJCOPY --add-gnu-debuglink="$link" "${stripped_file}" \
-		"${output_file}"
+		 "${output_file}"
     )
     rc=$?
     [ $rc != 0 ] && exit $rc
-- 
2.35.3

[PATCH v2 4/5] [gdb/testsuite] Clean up lib/dg-add-core-file-count.sh

[Tom de Vries]

Fix shellcheck warnings in script lib/dg-add-core-file-count.sh.

Tested on x86_64-linux.
---
 gdb/testsuite/lib/dg-add-core-file-count.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/dg-add-core-file-count.sh b/gdb/testsuite/lib/dg-add-core-file-count.sh
index 45cf0cf3bd5..b4cb6b93ea4 100755
--- a/gdb/testsuite/lib/dg-add-core-file-count.sh
+++ b/gdb/testsuite/lib/dg-add-core-file-count.sh
@@ -26,7 +26,7 @@
 # find, wc, etc.  Spawning a subshell isn't strictly needed, but it's
 # clearer.  The "*core*" pattern is this lax in order to find all of
 # "core", "core.PID", "core.<program>.PID", "<program>.core", etc.
-cores=$(set -- *core*; [ $# -eq 1 -a ! -e "$1" ] && shift; echo $#)
+cores=$(set -- *core*; [ $# -eq 1 ] && [ ! -e "$1" ] && shift; echo $#)
 
 # If no cores found, then don't add our summary line.
 if [ "$cores" -eq "0" ]; then
-- 
2.35.3

[PATCH v2 5/5] [gdb/build] Cleanup gdb/features/feature_to_c.sh

[Tom de Vries]

Clean up script gdb/features/feature_to_c.sh by:
- fixing shellcheck warnings,
- moving an embedded awk script out of the file, reducing the amount of
  escaping in the awk script, making it more readable and maintainable, and
- adding emacs / vi settings for local tab size 2 (copied from ./ltmain.sh).

Tested on x86_64-linux.
---
 gdb/features/feature_to_c.awk | 30 +++++++++++++++++++
 gdb/features/feature_to_c.sh  | 56 ++++++++++++++---------------------
 2 files changed, 52 insertions(+), 34 deletions(-)
 create mode 100644 gdb/features/feature_to_c.awk

diff --git a/gdb/features/feature_to_c.awk b/gdb/features/feature_to_c.awk
new file mode 100644
index 00000000000..42b9900f206
--- /dev/null
+++ b/gdb/features/feature_to_c.awk
@@ -0,0 +1,30 @@
+BEGIN { n = 0
+    printf "static const char %s[] = {\n", arrayname
+    for (i = 0; i < 255; i++)
+	_ord_[sprintf("%c", i)] = i
+}
+
+{
+    split($0, line, "");
+    printf "  "
+    for (i = 1; i <= length($0); i++) {
+	c = line[i]
+	if (c == "'") {
+	    printf "'\\''"
+	} else if (c == "\\") {
+	    printf "'\\\\'"
+	} else if (_ord_[c] >= 32 && _ord_[c] < 127) {
+	    printf "'%s'", c
+	} else {
+	    printf "'\\%03o'", _ord_[c]
+	}
+	printf ", "
+	if (i % 10 == 0)
+	    printf "\n   "
+    }
+    printf "'\\n', \n"
+}
+
+END {
+    print "  0 };"
+}
diff --git a/gdb/features/feature_to_c.sh b/gdb/features/feature_to_c.sh
index 3159ec523a6..d5d3db7157f 100755
--- a/gdb/features/feature_to_c.sh
+++ b/gdb/features/feature_to_c.sh
@@ -32,47 +32,35 @@ if test -e "$output"; then
   exit 1
 fi
 
-echo '#include "xml-builtin.h"' >> $output
+echo '#include "xml-builtin.h"' >> "$output"
+
+awk_script=$(echo "$0" | sed 's/\.sh$/.awk/')
 
 for input; do
-  arrayname=xml_feature_`echo $input | sed 's,.*/,,; s/[-.]/_/g'`
+  arrayname=xml_feature_$(echo "$input" | sed 's,.*/,,; s/[-.]/_/g')
 
-  ${AWK:-awk} 'BEGIN { n = 0
-      print "static const char '$arrayname'[] = {"
-      for (i = 0; i < 255; i++)
-        _ord_[sprintf("%c", i)] = i
-    } {
-      split($0, line, "");
-      printf "  "
-      for (i = 1; i <= length($0); i++) {
-        c = line[i]
-        if (c == "'\''") {
-          printf "'\''\\'\'''\'', "
-        } else if (c == "\\") {
-          printf "'\''\\\\'\'', "
-        } else if (_ord_[c] >= 32 && _ord_[c] < 127) {
-	  printf "'\''%s'\'', ", c
-        } else {
-          printf "'\''\\%03o'\'', ", _ord_[c]
-        }
-        if (i % 10 == 0)
-          printf "\n   "
-      }
-      printf "'\''\\n'\'', \n"
-    } END {
-      print "  0 };"
-    }' < $input >> $output
+  ${AWK:-awk} \
+    -v "arrayname=$arrayname" \
+    -f "$awk_script" \
+    < "$input" \
+    >> "$output"
 done
 
-echo >> $output
+echo >> "$output"
 
-echo "extern const char *const xml_builtin[][2] = {" >> $output
+echo "extern const char *const xml_builtin[][2] = {" >> "$output"
 
 for input; do
-  basename=`echo $input | sed 's,.*/,,'`
-  arrayname=xml_feature_`echo $input | sed 's,.*/,,; s/[-.]/_/g'`
-  echo "  { \"$basename\", $arrayname }," >> $output
+  basename=$(echo "$input" | sed 's,.*/,,')
+  arrayname=xml_feature_$(echo "$input" | sed 's,.*/,,; s/[-.]/_/g')
+  echo "  { \"$basename\", $arrayname }," >> "$output"
 done
 
-echo "  { 0, 0 }" >> $output
-echo "};" >> $output
+echo "  { 0, 0 }" >> "$output"
+echo "};" >> "$output"
+
+# Local Variables:
+# mode:shell-script
+# sh-indentation:2
+# End:
+# vi:sw=2
-- 
2.35.3

Re: [PATCH v2 1/5] [gdb/testsuite] Clean up gdb/contrib/expect-read1.sh

[Kevin Buettner]

All five patches in this set LGTM.

Approved-by: Kevin Buettner <kevinb@redhat.com>

Re: [PATCH v2 1/5] [gdb/testsuite] Clean up gdb/contrib/expect-read1.sh

[Tom de Vries]

On 6/14/24 20:32, Kevin Buettner wrote:
> All five patches in this set LGTM.
> 
> Approved-by: Kevin Buettner <kevinb@redhat.com>
> 

Hi Kevin,

thanks for the review, pushed.

Thanks,
- Tom

Re: [PATCH v2 5/5] [gdb/build] Cleanup gdb/features/feature_to_c.sh

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> - moving an embedded awk script out of the file, reducing the amount of
Tom>   escaping in the awk script, making it more readable and maintainable, and

The new file is missing the GPL header comment.

Tom

Re: [PATCH v2 5/5] [gdb/build] Cleanup gdb/features/feature_to_c.sh

[Tom de Vries]

On 6/15/24 20:15, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> - moving an embedded awk script out of the file, reducing the amount of
> Tom>   escaping in the awk script, making it more readable and maintainable, and
> 
> The new file is missing the GPL header comment.

Thanks for noticing, sorry about that.

I've submitted a fix here ( 
https://sourceware.org/pipermail/gdb-patches/2024-June/210044.html ).

Thanks,
- Tom