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

[Yao Qi <yao at codesourcery dot com>]

On 04/01/2014 07:10 AM, Pedro Alves wrote:
> +/* Returns true if the multi-process extensions are in effect.  */
> +static int
> +remote_multi_process_p (struct remote_state *rs)
> +{
> +  return packet_support (PACKET_multiprocess_feature) == PACKET_ENABLE;
> +}

Argument 'rs' is not needed.  We can remove it.
However, your patch moves more gdb remote target states from
'struct remote_state' to a global state.  I feel that it has
conflict with the multi-target project.  Not sure it is part
of your plan to convert the global state to per-target state.

> +# Test that setting a tracepoint while the trace experiment is ongoing
> +# doesn't work when we force-disable the InstallInTrace RSP feature.
> +
> +proc tracepoint_install_in_trace_disabled { trace_type } {
> +    with_test_prefix "3 $trace_type" {
> +	global testfile
> +	global srcfile
> +	global pcreg
> +	global gdb_prompt
> +
> +	clean_restart ${testfile}
> +	if ![runto_main] {
> +	    fail "Can't run to main"
> +	    return -1
> +	}
> +
> +	# This test only makes sense with the remote target.
> +	if ![gdb_is_target_remote] {
> +	    return
> +	}
> +
> +	gdb_test_no_output "delete break 1"
> +
> +	# Set a tracepoint we'll never meet.  Just to avoid the
> +	# complain after `tstart' later.
> +	gdb_test "next" ".*"
> +	gdb_test "trace main" \
> +	    "Tracepoint \[0-9\] at.* file .*$srcfile, line.*" \
> +	    "set tracepoint on main"
> +
> +	gdb_test_no_output "tstart"
> +
> +	# 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?  IMO, UNSUPPORTED is better.  Secondly, if "jump pad too far"
causes fast tracepoint not installed, we should return here to skip the
rest of the test, because the test below can't tell why a fast
tracepoint is not installed, is it caused by
"set remote install-in-trace-packet off" or "jump pad too far".

> +		} else {
> +		    fail $test
> +		}
> +	    }
> +	    -re "\r\n$gdb_prompt $" {
> +		pass $test
> +	    }
> +	}
> +
> +	gdb_trace_setactions "set action for tracepoint" "" \
> +	    "collect \$$pcreg" "^$"
> +
> +	# Make sure the tracepoint is _not_ installed on the target.
> +	gdb_test "info trace" \
> +	    "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
> +\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*3\.1.* in func4.*not installed on target.*3\.2.* in func4.*not installed on target.*" \
> +	    "tracepoint with two locations"
> +    }
> +}
> +

-- 
Yao (éå)