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

[Tom de Vries <tdevries@suse.de>]

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.

> +
> +    # 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.

Approved-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom


> +    return [expr $uid == 0]
> +}
> +
>   # Always load compatibility stuff.
>   load_lib future.exp