On 10/24/18 1:37 AM, Simon Marchi wrote:
> On 2018-10-23 6:38 p.m., Tom de Vries wrote:
>> On 10/23/18 11:05 PM, Tom de Vries wrote:
>>> On 10/23/18 11:04 PM, Simon Marchi wrote:
>>>> On 2018-10-15 3:54 p.m., Tom de Vries wrote:
>>>>>> Just wondering.  Would it make life easier if we fixed PR 23368, which
>>>>>> is the reason we have to do the test in an unnatural way?
>>>>>
>>>>> Yes.
>>>>
>>>> Hi Tom,
>>>>
>>>> PR 23368 should be fixed now.  Do you plan on updating catch-follow-exec.exp
>>>> to be written in a more standard way?
>>>
>>> Sure, will do.
>>
>> How does this look?
> 
> Hi Tom,
> 
> Thanks for looking into this so quickly.

And thanks for the quick review.

>  I have some superficial suggestions that
> can help shorten the test a bit and make it more readable (some of them can be personal
> preference though...).
> 
> When the test name is omitted, it defaults to the command.  So instead of
> 
>     gdb_test "catch exec" \
> 	{Catchpoint [0-9][0-9]* \(exec\)} \
> 	"catch exec"
> 
> You can write
> 
>     gdb_test "catch exec" {Catchpoint [0-9][0-9]* \(exec\)}
> 
> and the test name will be "catch exec".

Done.

>  Instead of [0-9][0-9]*, I am
> pretty sure you can use [0-9]+,

Done.

> or $decimal, which is provided by DejaGnu
> (/usr/share/dejagnu/runtest.exp):
> 
>   101:    set decimal "\[0-9\]+"
> 
> Except in the {} string, $decimal won't work, because it won't get
> substituted.

Indeed. I prefer the {} quoting over "" quoting if that means less
escaping, so I went with {} here.

> 
> For this:
> 
>     gdb_test "set follow-exec-mode new" \
> 	"" \
> 	"set follow-exec-mode new"
> 
> You can use
> 
>     gdb_test_no_output "set follow-exec-mode new"
> 

Done.

> (again, omitting the test name makes it default to the command)
> 
> I'd suggest replacing
> 
>     gdb_test_multiple "info prog" "info prog" {
> 	-i "$gdb_spawn_id" eof {
> 	    fail "info prog"
> 	}
> 	-i "$gdb_spawn_id" "No selected thread\."  {
> 	    pass "info prog"
> 	}
>     }
> 
> with the simpler
> 
>     gdb_test "info prog" "No selected thread."
> 
> If GDB crashes as it did before your fix, the test will be unresolved, which is
> treated the same as a FAIL.

Done.

> While at it, could you update the comment at the top of the file, which currently
> says:
> 
> # Check whether finish respects the print pretty user setting when printing the
> # function result.
> 

Done.

Also, I realized that by using runto_main at the start, I could replace
gdb_run_cmd/gdb_expect with a regular gdb_test continue.

Committed as attached.

Thanks,
- Tom