Re: [PATCH 1/4] New test for removing socket file in gdb.trace/strace.exp.

[Pedro Alves <palves@redhat.com>]

On 06/14/2012 03:39 PM, Yao Qi wrote:

> On Tuesday 12 June 2012 22:50:49 Pedro Alves wrote:
>>> +    set test "socket file exists"
>>> +    set socket_file "/tmp/gdb_ust${pid}"
>>> +    if { [file exists $socket_file] } {
>>> +     pass $test
>>> +    } else {
>>> +     fail $test
>>> +    }
>>
>> This won't work with remote host testing.  This file is really a
>> file on the target.  Why not use "remote_file target exists" ?
>>
> 
> Yes, we should use "remote_file XXX exits", however, I find "remote_file XXX
>  exists" always return 0 for socket file because it uses "[ -f $file ]" to check
>  whether file exists.  I work around this limitation in this way,
> 


Bummer.  :-(

>     set exists ""
>     if [is_remote target] {
>        set status [remote_exec target "sh -c \"exit `\\\[ -S $socket_file `\""]
>        set exists [expr [lindex $status 0] == 0]
>     } else {
>        set exists [file exists $socket_file]
>     }
> 
> b.t.w, the quote on tcl and bash makes crazy :)


:-)

In general, there are plenty of targets where we can't assume
a posix shell on the target.  But in this case, since this functionality
is implemented on GNU/Linux, we can go with it.  I think -S is a
bash extension, but again, it may be okay in this case.  I tried ksh,
and dash, and both accept it.

I don't understand how the else branch works for you with remote host
native testing, as you're checking for a file on the build machine, not
the host (where GDB runs) (unless you're sharing /tmp between
build and host machines, but you're probably not.)

Is there anything preventing using "remote_exec target" bits always,
getting rid of the else branch?

> 
>>> +
>>> +    send_gdb "${action}\n"
>>> +    gdb_expect {
>>> +     -re "A debugging session is active.\r\n.*\r\nQuit anyway\\? \\(y or
>>> n\\) $" { +         send_gdb "y\n"
>>> +     }
>>> +     -re "Detaching .*, process .*$" {
>>> +     }
>>> +     -re "Continuing.*$" {
>>> +     }
>>> +    }
>>
>> gdb_test_multiple ?
> 
> When I tried gdb_test_multiple, I'll get an extra fail, so I use
>  send_gdb/gdb_expect,
> 
>   FAIL: gdb.trace/strace.exp: remove_socket_after_quit: quit (got interactive prompt)
> 
> This new test is tested on native on local host, native-gdbserver, and native
>  on remote host.  


> I saw some problems when running test on remote host.  They are

> not related to this patch series, so I'd like to defer them to follow up
> fix later.


Thanks, that mode tends to rot a bit.

> 
> gdb/testsuite:
> 
> 2012-06-14  Yao Qi  <yao@codesourcery.com>
> 
> 	PR gdb/14161.
> 	* gdb.trace/strace.exp (strace_remove_socket): New proc to test
> 	the socket file is removed.


The uses of the new function should be mentioned as well.

> +proc strace_remove_socket { action } {


> +    sleep 5


We have 3 calls to this function, adding up to 15 seconds.

We could do things a bit differently to try to avoid always
sleeping, or sleeping so much.  For instance, with something like this
pseudo code:

 detach
 $exists=true
 for i in 0..4 {
     if ! socket exists
        $exists=false
        break
     sleep 1
 }
 if $exists
   fail $test
 else
   pass $test

IOW, check if the socket exists or not immediately after detaching.  Only
sleep if we find it still exists.  With luck, expect will be slow enough, and
we'll find the socket is gone on the first iteration, avoiding the sleep.
If not, we'll wait one second, then re-check, etc.

-- 
Pedro Alves