Re: [PATCH] [AArch64,v5] MTE corefile support

[Luis Machado <luis.machado@arm.com>]

Hi,

On 7/18/22 14:54, Pedro Alves wrote:
> Hi!
> 
> This version looks good to me.  Just one nit in the testcase.  No need to repost for this.
> 
> On 2022-07-11 11:57 a.m., Luis Machado wrote:
>> +foreach_with_prefix mode {"sync" "async"} {
>> +
>> +    if {$mode == "async"} {
>> +	lappend compile_flags "additional_flags=-DASYNC"
>> +    }
>> +
>> +    standard_testfile
>> +    set executable "${testfile}-${mode}"
>> +    if {[prepare_for_testing "failed to prepare" ${executable} ${srcfile} ${compile_flags}]} {
>> +	return -1
> 
> If we hit this return while testing "sync", then "async" won't be tested either.  Similarly
> for the other returns below.  To avoid scenarios like this (and in the future if the code evolves
> to have more early returns), it's IMO better/safer to move the body of the foreach_with_prefix
> to a procedure, like:
> 
>   proc test {mode} {
>       global srcfile testfile
>   
>       if {$mode == "async"} {
>   	lappend compile_flags "additional_flags=-DASYNC"
>       }
>   
>       standard_testfile
>       set executable "${testfile}-${mode}"
>       if {[prepare_for_testing "failed to prepare" ${executable} ${srcfile} ${compile_flags}]} {
>   	return -1
>   
>       ...
>   }
> 
>   foreach_with_prefix mode {"sync" "async"} {
>      test $mode
>   }

Thanks. That made it more organized. I've made the changes and pushed both the GDB and the binutils
changes.