Re: [PATCH v2] Fixes to gdb.mi testsuite

[Pedro Alves <palves at redhat dot com>]

On 06/18/2013 01:32 PM, Simon Marchi wrote:
> On 13-06-18 07:31 AM, Pedro Alves wrote:
>> On 06/17/2013 04:29 PM, Simon Marchi wrote:
>>> This is the updated version of the patch submitted in this thread:
>>> http://sourceware.org/ml/gdb-patches/2013-05/msg00971.html
>>>
>>> It contains two fixes to the gdb.mi testsuite:
>>
>> Note these are separate logical changes.  It'd be a little
>> better to keep them as separate patches/posts.
>>
>>> * Add support for args to mi_run_to_main (and the commands that it
>>>    depends on, mi_runto, mi_runto_helper). mi_run_cmd_full already
>>>    supported arguments, but it didn't work when use_mi_command was set.
>>
>> You don't need this anymore, right?
> 
> You are right. I think it is still a nice feature to have. 

Not sure.  Given the issue with passing arguments not working
everywhere, not supporting this on mi_run_to_main works as kind
of a deterrent from people doing that.  The CLI's runto_main also
doesn't take program args, and mi_run_to_main is documented as
"just like run-to-main for MI".

For the odd ball MI test that actually needs to pass args,
we can still use mi_run_cmd_full directly.

> Also mi_run_cmd_full already accepted an "args" parameter, but the behavior 
> is wrong when use_mi_command == 1, so that should be fixed.

Yes, that much I agree.

> -    send_gdb "${run_prefix}run $args\n"
> +    send_gdb "${set_args_cmd} $args\n"

It'd be better if the prompt was consumed before moving on to the
next command (to avoid the headache to the someone that changes the
following test in a way that'd be mysteriously broken if the prompt is
left in the expect buffer).  Based on other similar code in the function,
add something like:

	gdb_expect {
	    -re "$mi_gdb_prompt$" { }
	    default {
		perror "setting args failed"
		return -1
	    }

That part is OK with this change.

> +    send_gdb "${run_prefix}run\n"
>       gdb_expect {
>   	-re

Want to commit the mi_expect_stop part first, and then a patch
that just fixes mi_run_cmd_full?

Thanks,
-- 
Pedro Alves