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

Yao Qi
2014-04-01T12:48:53Z

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 (é½å°§)