Re: [PATCH] Fix several "set remote foo-packet on/off" commands.

[Yao Qi <yao at codesourcery dot com>]

On 04/01/2014 08:06 PM, Pedro Alves wrote:
> As part of that project, we'll definitely need to move the
> whole remote_protocol_packets global array to remote_state.
> This global array already exists.  I've just moved a few more manually
> managed flags into this array.  Note this is listed as a TODO in
> the MultiTarget project's wiki page.
> Therefore, I'm not really adding any conflict, as that work will
> already have to be done.  When that is done, we'll likely end up
> with passing a 'struct remote_state *' pointer to the new packet_support
> function.  And with that in mind, it just looks like unnecessary
> churn to remove the parameter from remote_multi_process_p now (and
> update all callers), only to add it back again.  Do you agree?
> 

Yes, that is fine to keep unused parameter 'rs' remote_multi_process_p.

>> > Not sure it is part
>> > of your plan to convert the global state to per-target state.
> I don't plan to work on it myself in the near future, but it's
> definitely the way to go.
> 

OK, got it.

>>> >> +
>>> >> +	# Force-disable the InstallInTrace RSP feature.
>>> >> +	gdb_test_no_output "set remote install-in-trace-packet off"
>>> >> +
>>> >> +	# Set a tracepoint while a trace experiment is ongoing.
>>> >> +	set test "set tracepoint on set_tracepoint"
>>> >> +	gdb_test_multiple "${trace_type} set_tracepoint" $test {
>>> >> +	    -re "Target returns error code .* too far .*$gdb_prompt $" {
>>> >> +		if [string equal $trace_type "ftrace"] {
>>> >> +		    # The target was unable to install the fast tracepoint
>>> >> +		    # (e.g., jump pad too far from tracepoint).
>>> >> +		    pass "$test (too far)"
>> > 
>> > A nit here, a fast tracepoint is not installed due to "jump pad too far"
>> > instead of "set remote install-in-trace-packet off".  Do we want to emit
>> > a PASS here?
> Oh, hmm.  Copy-paste from the other function in the file, and I
> just didn't think about that part much.  It actually makes no sense
> to even expect that the target sends back any error code.  We've just
> disabled installing tracepoints while a trace experiment is running,
> so we shouldn't see any response from the target at all.
> 
>> > IMO, UNSUPPORTED is better.
> Agreed, though note you made this a PASS in tracepoint_change_loc_1.  ;-)
> 

Sorry about that :)

patch v2 looks good to me.

-- 
Yao (éå)