Re: [PATCH v2] Fix argument passing in mi_run_cmd_full

[Simon Marchi <simon dot marchi at ericsson dot com>]

On Tue 20 May 2014 12:40:01 PM EDT, Simon Marchi wrote:
> On Mon 19 May 2014 03:39:09 PM EDT, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>>
>> Simon> 2014-05-13  Simon Marchi  <simon.marchi@ericsson.com>
>> Simon> 	* lib/mi-support.exp (mi_run_cmd_full): Set arguments by calling
>> Simon> 	"-exec-arguments" or "set args" before running the inferior.
>>
>> This patch causes a test failure here:
>
> Ehh, that's what I get for not re-running the testsuite, even after a 
> year. I will send something soon about that.

mi-start.exp passes "--start" to mi_run_cmd, which is intended to be an argument to -exec-run. My understanding was that the parameter "args" of mi_run_cmd_full was for arguments to pass to the test program (hence, my patch). A quick grep shows that mi-start.exp is the only place where arguments are actually passed to mi_run_cmd. At the time the patch was written, nowhere in the test suite were parameters passed to mi_run_cmd, so I just assumed that this was the intended behavior. I ended up not even using this fix, since Pedro suggested that to modify the behavior of a test program, I should stop at a breakpoint and modify the value of a global variable instead.

Perhaps the original behavior was not bad after all: $args is passed to the command used to run the program, "run" or "-exec-run", depending on the value of $use_mi_command. Therefore, the meaning/semantic of the $args parameter changes based on the value of $use_mi_command: arguments passed to the test program if false, arguments to -exec-run (such as --start) if true. If you use MI and want to pass arguments to the test program, then you need to manually call -exec-arguments before (or a helper proc could be created for that). If documented properly, I think this way is good.

If we decide to stay with the current behavior where $args always means the arguments to pass to the test program, then we'll need to add a separate parameter to mi_run_cmd_full, mi_run_cmd and mi_run_with_cli to convey the arguments to pass to the run command (to be able to pass the --start). This makes things a little bit messy:

    proc mi_run_cmd {args}

will become

    proc mi_run_cmd {test_program_args run_command_args}

which means that most calls to mi_run_cmd will have to be changed from

    mi_run_cmd

to

    mi_run_cmd "" ""

because of the special vararg-like meaning of the parameter named "args".

In my opinion, I think the old behavior was fine, it just needs to be documented better (clarify what $args means in the mi_run_* procedures).

I would like to hear what you think about this.

Thanks,

Simon