Re: [PATCH v2] GDB/testsuite: Add a way to send multiple init commands

[Pedro Alves <palves at redhat dot com>]

On 07/10/2014 01:17 AM, Maciej W. Rozycki wrote:
> On Fri, 20 Jun 2014, Pedro Alves wrote:
> 
>>> 2014-06-11  Maciej W. Rozycki  <macro@mips.com>
>>>             Maciej W. Rozycki  <macro@codesourcery.com>
>>>
>>> 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
>>> 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
>>> 	* README (Board Settings): Document `gdb_init_command' and 
>>> 	`gdb_init_commands'.
>>
>> I don't particularly see much need for this -- I do this in my
>> boards instead:
>>
>>  set GDBFLAGS ""
>>  set GDBFLAGS "${GDBFLAGS} -ex \"set breakpoint always-inserted on\""
>>  set GDBFLAGS "${GDBFLAGS} -ex \"set target-async 1\""
>>
>> See:
>>
>> https://sourceware.org/gdb/wiki/TestingGDB#Passing_an_option_to_GDB_.2BAC8_Running_the_whole_test_suite_in_a_non-default_mode
>>
>> But, given gdb_init_command exists and this can be made
>> non-intrusive, it's fine with me to add the new option.
> 
>  That and I think there are two issues with passing commands as 
> command-line arguments:
> 
> 1. They are always executed, perhaps unnecessarily whereas 
>    `gdb_init_command' and consequently `gdb_init_commands' are only 
>    interpreted when a target connection is about to be made (this is more 
>    of an aesthetic matter, but still).

Which I'll guess is an historic accident (not by design) given the
option's name.  I'd think hooking gdb_reload/gdb_load would do just
as well.

> 
> 2. Some environments have a limit, maybe quite low, on the maximum length 
>    of a command line or command-line arguments they accept (now that is 
>    more real).

Not really an issue, as you can just put the commands in a GDB script
and do:

  set GDBFLAGS "${GDBFLAGS} -x \"/path/to/script.gdb\""

Very much like a response file.

> 
>  BTW, in updating DejaGNU documentation that refers to `gdb_init_command' 
> I've noticed it lists a command that pokes at a CPU register there -- has 
> the semantics of the setting changed sometime, perhaps long ago?  Does 
> anybody know/remember?

I don't know the history, but I'd guess it's related to
this in config/gdb-comm.exp (found in dejagnu itself, not
gdb):

#
# gdb_comm_load -- load the program and execute it
#
# PROG is a full pathname to the file to load, no arguments.
# Result is "untested", "pass", "fail", etc.
#
proc gdb_comm_load { dest prog args } {
...
    remote_send host "target $protocol $targetname\n"
    remote_expect host 60 {
...
    if {[target_info exists gdb_init_command]} {
	remote_send host "[target_info gdb_init_command]\n"
	remote_expect host 10 {
	    -re ".*$gdb_prompt $" { }
	    default {
		gdb_comm_leave
		return [list "fail" ""]
	    }
	}
    }
...
}

So in that board, gdb_init_command runs after connecting, and
is used to configure the board after connecting.  Clearly there's
a usage conflict here...

(that's a ${board}_load override, note.)

I'd guess most of what's in that file predates all the equivalent
infrastructure we have, even.  Maybe gdb_init_command started out
there before in appeared in gdb's codebase.  But that's just
guesswork.  I wasn't around then.  :-)  I have no idea if gdb-comm.exp
is still used.

>  Done, as below, and retested.  Any other questions or comments?  
> Otherwise OK to apply?

This version looks good to me.

Thanks,
-- 
Pedro Alves