[patch] gdb_unload - account for another termination message

[patch] gdb_unload - account for another termination message

[Phil Muldoon]

While working on another bug I attempted to use gdb_unload in the
testsuite.  I noticed that it did not account for a termination
message. (This was the one message I was getting).  This would lead to a
timeout and failure.   Is it ok to just add another -re test in the
original test here?

Cheers

Phil

2011-03-15  Phil Muldoon  <pmuldoon@redhat.com>

	* lib/gdb.exp (gdb_unload): Add another termination case.

--

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 1b601af..6cc8410 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -142,6 +142,11 @@ proc gdb_unload {} {
 		verbose "\t\tKilling previous program being debugged"
 	    exp_continue
 	}
+	-re "A program is being debugged already..*Are you sure.*y or n. $"\
+	    { send_gdb "y\n"
+		verbose "\t\tKilling previous program being debugged"
+	    exp_continue
+	}
 	-re "Discard symbol table from .*y or n.*$" {
 	    send_gdb "y\n"
 	    exp_continue

Re: [patch] gdb_unload - account for another termination message

[Pedro Alves]

On Tuesday 15 March 2011 10:56:31, Phil Muldoon wrote:
> 
> While working on another bug I attempted to use gdb_unload in the
> testsuite.  I noticed that it did not account for a termination
> message. (This was the one message I was getting).  This would lead to a
> timeout and failure.   Is it ok to just add another -re test in the
> original test here?

The current code reads:

proc gdb_unload {} {
    global verbose
    global GDB
    global gdb_prompt
    send_gdb "file\n"
    gdb_expect 60 {
	-re "No executable file now\[^\r\n\]*\[\r\n\]" { exp_continue }
	-re "No symbol file now\[^\r\n\]*\[\r\n\]" { exp_continue }
	-re "A program is being debugged already..*Kill it.*y or n. $"\
	    { send_gdb "y\n"
		verbose "\t\tKilling previous program being debugged"
	    exp_continue
	}

I don't think the "Kill it" message can ever appear in response
to a "file" command.  This may be old behavior.  I'd rather we
replace this part with current reality.

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 1b601af..6cc8410 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -142,6 +142,11 @@ proc gdb_unload {} {
>  		verbose "\t\tKilling previous program being debugged"
>  	    exp_continue
>  	}
> +	-re "A program is being debugged already..*Are you sure.*y or n. $"\
> +	    { send_gdb "y\n"
> +		verbose "\t\tKilling previous program being debugged"

The full message I see is:

(top-gdb) file gdb
A program is being debugged already.
Are you sure you want to change the file? (y or n) 

So please adjust the regex to catch the whole frase,
not just the "Are you sure" part, which could paper
over other questions.  Also, "Killing previous program..." isn't
what answering 'y' does.


> +	    exp_continue
> +	}
>  	-re "Discard symbol table from .*y or n.*$" {
>  	    send_gdb "y\n"
>  	    exp_continue
> 

-- 
Pedro Alves

Re: [patch] gdb_unload - account for another termination message

[Phil Muldoon]

Pedro Alves <pedro@codesourcery.com> writes:

> On Tuesday 15 March 2011 10:56:31, Phil Muldoon wrote:
>> 
>> While working on another bug I attempted to use gdb_unload in the
>> testsuite.  I noticed that it did not account for a termination
>> message. (This was the one message I was getting).  This would lead to a
>> timeout and failure.   Is it ok to just add another -re test in the
>> original test here?
>
> The current code reads:
>
> proc gdb_unload {} {
>     global verbose
>     global GDB
>     global gdb_prompt
>     send_gdb "file\n"
>     gdb_expect 60 {
> 	-re "No executable file now\[^\r\n\]*\[\r\n\]" { exp_continue }
> 	-re "No symbol file now\[^\r\n\]*\[\r\n\]" { exp_continue }
> 	-re "A program is being debugged already..*Kill it.*y or n. $"\
> 	    { send_gdb "y\n"
> 		verbose "\t\tKilling previous program being debugged"
> 	    exp_continue
> 	}
>
> I don't think the "Kill it" message can ever appear in response
> to a "file" command.  This may be old behavior.  I'd rather we
> replace this part with current reality.


Yeah I just wasn't sure here, I did the grep before I wrote the patch:

[pmuldoon@localhost gdb]$ egrep -irs "A program is being debugged" *.c
exec.c:326:      && !query (_("A program is being debugged already.\n"
infcmd.c:2433:      if (query (_("A program is being debugged already.  Kill it? ")))
target.c:2446:	  || query (_("A program is being debugged already.  Kill it? ")))

And there are three exit points.  I thought briefly about standardizing
the message.  Anyway, I'll replace the old message in gdb_unload.


> The full message I see is:
>
> (top-gdb) file gdb
> A program is being debugged already.
> Are you sure you want to change the file? (y or n) 
>
> So please adjust the regex to catch the whole frase,
> not just the "Are you sure" part, which could paper
> over other questions.  Also, "Killing previous program..." isn't
> what answering 'y' does.

From my previous grep that could be the only response.  But I take your
point well (and caution here costs nothing).  I see no need for the
verbose, tbh.  I'm not sure why I put it there.

--

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 1b601af..0d29428 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -137,9 +137,8 @@ proc gdb_unload {} {
     gdb_expect 60 {
        -re "No executable file now\[^\r\n\]*\[\r\n\]" { exp_continue }
        -re "No symbol file now\[^\r\n\]*\[\r\n\]" { exp_continue }
-       -re "A program is being debugged already..*Kill it.*y or n. $"\
-           { send_gdb "y\n"
-               verbose "\t\tKilling previous program being debugged"
+       -re "A program is being debugged already.*Are you sure you want to change the file?.*y or n. $" {
+           send_gdb "y\n"
            exp_continue
        }
        -re "Discard symbol table from .*y or n.*$" {

Re: [patch] gdb_unload - account for another termination message

[Pedro Alves]

On Tuesday 15 March 2011 12:50:58, Phil Muldoon wrote:
> +       -re "A program is being debugged already.*Are you sure you want to change the file?.*y or n. $" {
> +           send_gdb "y\n"
>             exp_continue
>         }

'?' in a regex has special meaning.  You want to escape it, replace
it with '.' (xxx file. .y or n.),  or just remove it, as the .* will
eat it.

Otherwise okay, thanks.

-- 
Pedro Alves

Re: [patch] gdb_unload - account for another termination message

[Phil Muldoon]

Pedro Alves <pedro@codesourcery.com> writes:

> On Tuesday 15 March 2011 12:50:58, Phil Muldoon wrote:
>> +       -re "A program is being debugged already.*Are you sure you want to change the file?.*y or n. $" {
>> +           send_gdb "y\n"
>>             exp_continue
>>         }
>
> '?' in a regex has special meaning.  You want to escape it, replace
> it with '.' (xxx file. .y or n.),  or just remove it, as the .* will
> eat it.
>
> Otherwise okay, thanks.

Thanks.  Committed with changes.

Cheers

Phil