Re: [PATCH 2/2] gdb-add-index: fix shellcheck warnings

[Simon Marchi <simark@simark.ca>]

On 3/29/25 7:42 PM, Keith Seitz wrote:
> Hi,
> 
> On 3/28/25 11:58 AM, Sam James wrote:
>> * Drop useless echo
>> * Add missing quoting to rm argument (and use -- to delimit options)
>> * Fix quoting w/ trap to avoid expanding $tmp_files immediately
> 
> Again, thank you for doing this.
> 
>> ---
>>   gdb/contrib/gdb-add-index.sh | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
>> index b299f830373..f727d28aac7 100755
>> --- a/gdb/contrib/gdb-add-index.sh
>> +++ b/gdb/contrib/gdb-add-index.sh
>> @@ -163,10 +162,10 @@ for f in "$file" "$dwz_file"; do
>>       tmp_files="$tmp_files $index4 $index5 $debugstr $debugstrmerge $debugstrerr"
>>   done
>>   -rm -f $tmp_files
>> +rm -f -- "$tmp_files"
> 
> By preventing expansion, wouldn't this be attempting to
> erase a single file whose name is a concatenation of all the
> temporary files?
> 
> That is,
> $ touch a b c
> $ tmp="a b c"
> $ rm -- "$tmp"
> rm: cannot remove 'a b c': No such file or directory
> 
> Keith
> 

Indeed, this looks wrong.  But not quoting it is also a problem if there
are spaces in the paths.  I think a sane way to handle this would be to
require bash and use arrays [1].

Simon

[1] https://www.gnu.org/software/bash/manual/html_node/Arrays.html