[PATCH 0/2] [gdb] Two regformats/regdat.sh fixes

[PATCH 0/2] [gdb] Two regformats/regdat.sh fixes

[Tom de Vries]

The first patch makes the script shellcheck clean.

The second patch fixes an unbound variable error.

Tom de Vries (2):
  [gdb] Make regformats/regdat.sh shellcheck-clean
  [gdb] Fix unbound variable in regformats/regdat.sh

 gdb/contrib/shellcheck.sh |  1 -
 gdb/regformats/regdat.sh  | 35 ++++++++++++++++++-----------------
 2 files changed, 18 insertions(+), 18 deletions(-)


base-commit: 1469a04f30917ef6a2bf4afc73e442b836574563
-- 
2.51.0

[PATCH 1/2] [gdb] Make regformats/regdat.sh shellcheck-clean

[Tom de Vries]

Make regformats/regdat.sh shellcheck-clean:
- use read -r
- use $()
- ignore intentional word-splitting
- add missing quotes
- use $(())

Handle a shellcheck error in this:
...
  if eval test \"\${${r}}\" = \"\ \"
...
by simplifying to:
...
  eval "rvalue=\$$r"
  if test "${rvalue:-}" = " "
...

Note that shellcheck can't detect that rvalue is assigned to, so we use
'${parameter:-word}' to use default value "", to silence a SC2154 [1].

Also, handle a shellcheck error in this:
...
  eval ${r}=""
...
by simplifying to:
...
  eval "$r=''"
...

Tested on x86_64-linux using:
...
$ for f in $(find gdb/regformats -name "*.dat"); do \
      sh gdb/regformats/regdat.sh \
          $f \
	  gdbsupport/osabi.def \
	  $(echo $f | sed 's%/%-%g'); \
  done
...
and comparing the generated files with and without this patch.

[1] https://www.shellcheck.net/wiki/SC2154
---
 gdb/contrib/shellcheck.sh |  1 -
 gdb/regformats/regdat.sh  | 25 ++++++++++++++-----------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/gdb/contrib/shellcheck.sh b/gdb/contrib/shellcheck.sh
index f7f7bf9efbf..a3e80a553f4 100755
--- a/gdb/contrib/shellcheck.sh
+++ b/gdb/contrib/shellcheck.sh
@@ -47,7 +47,6 @@ for f in "$@"; do
 	    | gdb/contrib/gdb-add-index.sh \
 	    | gdb/gdb_buildall.sh \
 	    | gdb/gdb_mbuild.sh \
-	    | gdb/regformats/regdat.sh \
 	    | gdb/testsuite/lib/pdtrace.in)
 	    # Skip unclean files.
 	    continue
diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index b57b76132aa..01f2dabe459 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -25,7 +25,7 @@ do_read ()
 {
     type=""
     entry=""
-    while read line
+    while read -r line
     do
 	if test "${line}" = ""
 	then
@@ -42,9 +42,11 @@ ${line}"
 	    # The semantics of IFS varies between different SH's.  Some
 	    # treat ``::' as three fields while some treat it as just too.
 	    # Work around this by eliminating ``::'' ....
-	    line="`echo "${line}" | sed -e 's/::/: :/g' -e 's/::/: :/g'`"
+	    line="$(echo "${line}" | sed -e 's/::/: :/g' -e 's/::/: :/g')"
 
 	    OFS="${IFS}" ; IFS="[:]"
+	    # Word-splitting on read variable is required.
+	    # shellcheck disable=SC2086
 	    eval read ${read} <<EOF
 ${line}
 EOF
@@ -54,9 +56,10 @@ EOF
 	    # that ended up with just that space character.
 	    for r in ${read}
 	    do
-		if eval test \"\${${r}}\" = \"\ \"
+		eval "rvalue=\$$r"
+		if test "${rvalue:-}" = " "
 		then
-		    eval ${r}=""
+		    eval "$r=''"
 		fi
 	    done
 
@@ -71,7 +74,7 @@ EOF
     fi
 }
 
-if test ! -r $1; then
+if test ! -r "$1"; then
   echo "$0: Could not open $1." 1>&2
   exit 1
 fi
@@ -105,8 +108,8 @@ EOF
 }
 
 
-exec > new-$3
-copyright $1
+exec > new-"$3"
+copyright "$1"
 echo '#include "regdef.h"'
 echo '#include "tdesc.h"'
 echo
@@ -119,7 +122,7 @@ xmlosabi=x
 expedite=x
 feature=x
 osabi=unknown
-exec < $1
+exec < "$1"
 while do_read
 do
   if test "${type}" = "name"; then
@@ -162,13 +165,13 @@ do
     echo "  tdesc_create_reg (feature, \"${entry}\","
     echo "  0, 0, NULL, ${type}, NULL);"
 
-    offset=`expr ${offset} + ${type}`
-    i=`expr $i + 1`
+    offset=$((offset + type))
+    i=$((i + 1))
   fi
 done
 
 echo
-echo "static const char *expedite_regs_${name}[] = { \"`echo ${expedite} | sed 's/,/", "/g'`\", 0 };"
+echo "static const char *expedite_regs_${name}[] = { \"$(echo "${expedite}" | sed 's/,/", "/g')\", 0 };"
 
 echo "#ifndef IN_PROCESS_AGENT"
 if test "${feature}" != x; then
-- 
2.51.0

[PATCH 2/2] [gdb] Fix unbound variable in regformats/regdat.sh

[Tom de Vries]

While testing the previous patch, I noticed the following difference:
...
$ sh ./gdb/regformats/regdat.sh \
      gdb/regformats/rs6000/powerpc-isa205-ppr-dscr-vsx64l.dat \
      gdbsupport/osabi.def \
      tmp.txt
$ ./gdb/regformats/regdat.sh \
      gdb/regformats/rs6000/powerpc-isa205-ppr-dscr-vsx64l.dat \
      gdbsupport/osabi.def \
      tmp.txt
./gdb/regformats/regdat.sh: line 33: comment: unbound variable
...

The difference is due to the start of regdat.sh:
...
 #!/bin/sh -u
...
where -u is enabling the unbound variable check.

In the first variant, this setting is ignored.

Fix this by using "set -u" instead, making sure that the unbound variable
check is done for both variants.

The comment variable is not used in a meaningful way, so fix the unbound
variable error by removing it.

Tested in the same way as the preceding patch.
---
 gdb/regformats/regdat.sh | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index 01f2dabe459..f93270ba0ca 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -1,4 +1,4 @@
-#!/bin/sh -u
+#!/bin/sh
 
 # Register protocol definitions for GDB, the GNU debugger.
 # Copyright (C) 2001-2026 Free Software Foundation, Inc.
@@ -18,6 +18,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+set -u
+
 # Format of the input files
 read="type entry"
 
@@ -28,15 +30,11 @@ do_read ()
     while read -r line
     do
 	if test "${line}" = ""
-	then
-	    continue
-	elif test "${line}" = "#" -a "${comment}" = ""
 	then
 	    continue
 	elif expr "${line}" : "#" > /dev/null
 	then
-	    comment="${comment}
-${line}"
+	    continue
 	else
 
 	    # The semantics of IFS varies between different SH's.  Some
-- 
2.51.0

Re: [PATCH 1/2] [gdb] Make regformats/regdat.sh shellcheck-clean

[Simon Marchi]

On 2026-08-28 08:04, Tom de Vries wrote:
> Make regformats/regdat.sh shellcheck-clean:
> - use read -r
> - use $()
> - ignore intentional word-splitting
> - add missing quotes
> - use $(())
> 
> Handle a shellcheck error in this:
> ...
>   if eval test \"\${${r}}\" = \"\ \"
> ...
> by simplifying to:
> ...
>   eval "rvalue=\$$r"
>   if test "${rvalue:-}" = " "
> ...
> 
> Note that shellcheck can't detect that rvalue is assigned to, so we use
> '${parameter:-word}' to use default value "", to silence a SC2154 [1].
> 
> Also, handle a shellcheck error in this:
> ...
>   eval ${r}=""
> ...
> by simplifying to:
> ...
>   eval "$r=''"
> ...
> 
> Tested on x86_64-linux using:
> ...
> $ for f in $(find gdb/regformats -name "*.dat"); do \
>       sh gdb/regformats/regdat.sh \
>           $f \
> 	  gdbsupport/osabi.def \
> 	  $(echo $f | sed 's%/%-%g'); \
>   done
> ...
> and comparing the generated files with and without this patch.

LGTM, thanks.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Random thought: I wonder why we don't check in the result of running
regdat.sh in the repo, like we do for gdbarch-gen.{c,h},
target-delegates-gen.c, and the features directory.  The output doesn't
change from build to build, and that would make it easy to see that your
patch did not produce any change in the output files.

Simon

Re: [PATCH 2/2] [gdb] Fix unbound variable in regformats/regdat.sh

[Simon Marchi]

On 2026-08-28 08:04, Tom de Vries wrote:
> While testing the previous patch, I noticed the following difference:
> ...
> $ sh ./gdb/regformats/regdat.sh \
>       gdb/regformats/rs6000/powerpc-isa205-ppr-dscr-vsx64l.dat \
>       gdbsupport/osabi.def \
>       tmp.txt
> $ ./gdb/regformats/regdat.sh \
>       gdb/regformats/rs6000/powerpc-isa205-ppr-dscr-vsx64l.dat \
>       gdbsupport/osabi.def \
>       tmp.txt
> ./gdb/regformats/regdat.sh: line 33: comment: unbound variable
> ...
> 
> The difference is due to the start of regdat.sh:
> ...
>  #!/bin/sh -u
> ...
> where -u is enabling the unbound variable check.
> 
> In the first variant, this setting is ignored.
> 
> Fix this by using "set -u" instead, making sure that the unbound variable
> check is done for both variants.
> 
> The comment variable is not used in a meaningful way, so fix the unbound
> variable error by removing it.
> 
> Tested in the same way as the preceding patch.

LGTM, thanks.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

> ---
>  gdb/regformats/regdat.sh | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
> index 01f2dabe459..f93270ba0ca 100755
> --- a/gdb/regformats/regdat.sh
> +++ b/gdb/regformats/regdat.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh -u
> +#!/bin/sh
>  
>  # Register protocol definitions for GDB, the GNU debugger.
>  # Copyright (C) 2001-2026 Free Software Foundation, Inc.
> @@ -18,6 +18,8 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> +set -u
> +
>  # Format of the input files
>  read="type entry"
>  
> @@ -28,15 +30,11 @@ do_read ()
>      while read -r line
>      do
>  	if test "${line}" = ""
> -	then
> -	    continue
> -	elif test "${line}" = "#" -a "${comment}" = ""
>  	then
>  	    continue
>  	elif expr "${line}" : "#" > /dev/null
>  	then
> -	    comment="${comment}

Unrelated comment: I'd like if we could standardize on

  if ...; then
  for ... in ...; do
  while ...; do

instead of

  if ...
  then

  for ... in ...
  do

  while ...
  do

I think it's more common, easier to read and takes less unnecessary
lines.

Simon

Re: [PATCH 1/2] [gdb] Make regformats/regdat.sh shellcheck-clean

[Andreas Schwab]

On Aug 28 2026, Tom de Vries wrote:

> +		    eval "$r=''"

You don't need the quotes on the rhs.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH 1/2] [gdb] Make regformats/regdat.sh shellcheck-clean

[Tom de Vries]

On 8/28/26 3:34 PM, Andreas Schwab wrote:
> On Aug 28 2026, Tom de Vries wrote:
> 
>> +		    eval "$r=''"
> 
> You don't need the quotes on the rhs.
> 

True, I just like the explicit rhs.

Thanks,
- Tom

Re: [PATCH 2/2] [gdb] Fix unbound variable in regformats/regdat.sh

[Tom de Vries]

On 8/28/26 2:46 PM, Simon Marchi wrote:
> Unrelated comment: I'd like if we could standardize on
> 
>    if ...; then
>    for ... in ...; do
>    while ...; do
> 
> instead of
> 
>    if ...
>    then
> 
>    for ... in ...
>    do
> 
>    while ...
>    do
> 
> I think it's more common, easier to read and takes less unnecessary
> lines.

Agreed.  I almost started doing this manually, but suddenly remembered 
that pretty-printers exist.

I've used shfmt ( 
https://sourceware.org/pipermail/gdb-patches/2026-August/229822.html ).

I also tried out beautysh (which would have been easy to integrate into 
pre-commit, because it's python), but it doesn't seem to fix this style.

I also tried out bash --pretty-print, but it has run-away indentation 
and doesn't seem to support customization.

Thanks,
- Tom

Re: [PATCH 1/2] [gdb] Make regformats/regdat.sh shellcheck-clean

[Tom de Vries]

On 8/28/26 2:43 PM, Simon Marchi wrote:
> Random thought: I wonder why we don't check in the result of running
> regdat.sh in the repo, like we do for gdbarch-gen.{c,h},
> target-delegates-gen.c, and the features directory.  The output doesn't
> change from build to build, and that would make it easy to see that your
> patch did not produce any change in the output files.

Good question, hadn't thought about it.

I suppose not checking in an intermediate result means you never use 
something stale, so I guess it comes down to how you want to prevent that.

Thanks,
- Tom