[gdb/contrib] Simplify rc usage in cc-with-tweaks.sh
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
Commit Message
The usage of the rc variable in gdb/contrib/cc-with-tweaks.sh follows a
certain pattern:
...
$ egrep '(rc=|\$rc)' gdb/contrib/cc-with-tweaks.sh | grep -v cmp_rc
rc=$?
[ $rc != 0 ] && exit $rc
rc=$?
[ $rc != 0 ] && exit $rc
rc=${PIPESTATUS[0]}
[ "$rc" != 0 ] && exit "$rc"
rc=$?
[ $rc != 0 ] && exit $rc
rc=0
rc=$?
[ $rc != 0 ] && exit $rc
rc=$?
[ $rc != 0 ] && exit $rc
rc=$?
[ $rc != 0 ] && exit $rc
rc=$?
[ $rc != 0 ] && exit $rc
exit "$rc"
...
It is:
- set using 'rc=$?' or 'rc=${PIPESTATUS[0]}', and then
- used in '[ $rc != 0 ] && exit $rc'.
The two exceptions are 'rc=0', and 'exit "$rc"'.
Remove the 'rc=0', there's no path reaching it where it's not already 0.
Removing the 'exit "$rc"' changes semantics in a cornercase.
Consider:
...
if true; then
rc=0
[ $rc != 0 ] && exit $rc
fi
exit $rc
...
What happens is:
- the if block is entered
- rc is set to 0
- the test evaluates to 1, so the exit doesn't trigger
- the if returns status of last command (1)
- script exits with 0
If we remove the exit, we have:
- the if block is entered
- rc is set to 0
- the test evaluates to 1, so the exit doesn't trigger
- the if returns status of last command (1)
- script exits with status of last command (1)
Fix this using:
...
-[ $rc != 0 ] && exit $rc
+if [ $rc != 0 ]; then exit $rc; fi
...
---
gdb/contrib/cc-with-tweaks.sh | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
base-commit: 62b131d7aebdb9921f47656cb0590f99fc31edbf
Comments
On 9/2/26 9:15 PM, Tom de Vries wrote:
> Fix this using:
> ...
> -[ $rc != 0 ] && exit $rc
> +if [ $rc != 0 ]; then exit $rc; fi
> ...
I've pushed this, with one change in the code: add a final "exit 0".
I've also changed the rationale for removing rc=0 to "unused def".
Thanks,
- Tom
@@ -163,7 +163,7 @@ output_dir="${output_file%/*}"
"$@"
rc=$?
-[ $rc != 0 ] && exit $rc
+if [ $rc != 0 ]; then exit $rc; fi
if [ ! -f "$output_file" ]
then
echo "$myname: Internal error: $output_file missing." >&2
@@ -184,7 +184,7 @@ get_tmpdir ()
if [ "$want_objcopy_compress" = true ]; then
$OBJCOPY --compress-debug-sections "$output_file"
rc=$?
- [ $rc != 0 ] && exit $rc
+ if [ $rc != 0 ]; then exit $rc; fi
fi
if [ "$want_index" = true ]; then
@@ -212,7 +212,7 @@ if [ "$want_index" = true ]; then
rc=${PIPESTATUS[0]}
mv "$tmpfile" "$output_file"
rm -f "$tmpdir"/*.dwo
- [ "$rc" != 0 ] && exit "$rc"
+ if [ "$rc" != 0 ]; then exit "$rc"; fi
fi
if [ "$want_index_cache" = true ]; then
@@ -221,7 +221,7 @@ if [ "$want_index_cache" = true ]; then
-ex "set index-cache enabled on" \
-ex "file $output_file"
rc=$?
- [ $rc != 0 ] && exit $rc
+ if [ $rc != 0 ]; then exit $rc; fi
fi
if [ "$want_dwz" = true ] || [ "$want_multi" = true ]; then
@@ -290,11 +290,10 @@ if [ "$want_dwp" = true ]; then
| sed -e 's/^.*: //' \
| sort \
| uniq)
- rc=0
if [ ${#dwo_files[@]} -ne 0 ]; then
$DWP -o "${output_file}.dwp" "${dwo_files[@]}" > /dev/null
rc=$?
- [ $rc != 0 ] && exit $rc
+ if [ $rc != 0 ]; then exit $rc; fi
rm -f "${dwo_files[@]}"
fi
fi
@@ -313,11 +312,11 @@ if [ "$want_gnu_debuglink" = true ]; then
strip "${STRIP_ARGS_STRIP_DEBUG[@]}" "${output_file}" \
-o "${stripped_file}"
rc=$?
- [ $rc != 0 ] && exit $rc
+ if [ $rc != 0 ]; then exit $rc; fi
strip "${STRIP_ARGS_KEEP_DEBUG[@]}" "${output_file}" \
-o "${debug_file}"
rc=$?
- [ $rc != 0 ] && exit $rc
+ if [ $rc != 0 ]; then exit $rc; fi
# The .gnu_debuglink is supposed to contain no leading directories.
link=$(basename "${debug_file}")
@@ -332,7 +331,5 @@ if [ "$want_gnu_debuglink" = true ]; then
"${output_file}"
)
rc=$?
- [ $rc != 0 ] && exit $rc
+ if [ $rc != 0 ]; then exit $rc; fi
fi
-
-exit "$rc"