Re: [PATCH v3] gdb/testsuite: XFAIL some gdb.base/fileio.exp

[Guinevere Larsen <blarsen@redhat.com>]

On 04/10/2023 17:33, Tom de Vries wrote:
> On 10/4/23 16:02, Guinevere Larsen via Gdb-patches wrote:
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index de22da8d8a8..5b90b8af3b8 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -9981,5 +9981,25 @@ gdb_caching_proc have_system_header { file } {
>>       return [gdb_can_simple_compile have_system_header_$name $src 
>> object]
>>   }
>>   +# Return 1 if the test is being run as root, 0 otherwise.
>> +
>> +gdb_caching_proc root_user {} {
>> +    # ID outputs to stdout, we have to use exec to capture it here.
>> +    set user [remote_exec target id]
>> +
>> +    regexp -all "(-?\[0-9\]+).*" $user user ret_val
>
> I can see that this works, but it's a bit unusual (and hard to parse 
> for me) to use regexp to pick apart a list. The usual way is using 
> lindex.

That's a good point.

TCL lists confuse me a lot, and the original plan was to have this along 
with the uid picking part but that failed for other reasons when the 
command failed.

>
>> +
>> +    # If ret_val is not 0, we couldn't run `id` on the target for some
>> +    # reason.  Return that we are not root, so problems are easier to
>> +    # spot.
>> +    if {[expr $ret_val != 0]} {
>
> The expr is not necessary here.
>
>> +    return 0
>> +    }
>> +
>> +    regexp -all ".*uid=(\[0-9\]+).*" $user user uid
>> +
>
> Also, the 'user' variable is used as dummy, better make that explicit.
>
> So, please do:
> ...
>     set res [remote_exec target id]
>     set ret_val [lindex $res 0]
>     set output [lindex $res 1]
>
>     # If ret_val is not 0, we couldn't run `id` on the target for some
>     # reason.  Return that we are not root, so problems are easier to
>     # spot.
>     if { $ret_val != 0 } {
>         return 0
>     }
>
>     regexp -all ".*uid=(\[0-9\]+).*" $output dummy uid
>
>     return [expr $uid == 0]
> ...
>
> Approved with that change.
Thanks! I applied your change and pushed it.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> Approved-By: Tom de Vries <tdevries@suse.de>
>
> Thanks,
> - Tom
>
>
>> +    return [expr $uid == 0]
>> +}
>> +
>>   # Always load compatibility stuff.
>>   load_lib future.exp
>