Hi Pedro,

Thanks for your help.

On 03/29/12 19:36, Pedro Alves wrote:
> On 03/29/2012 02:17 AM, Hui Zhu wrote:
>
>> Hi Tom,
>>
>> Thanks for your review.
>> I post a new patch according to your comments.
>>
>> Best,
>> Hui
>>
>> 2012-03-29  Hui Zhu<hui_zhu@mentor.com>
>>
>>      * gdb.trace/Makefile.in (BUILD_DATA_DIRECTORY): Add
>
>
> "BUILD_DATA_DIRECTORY" ?  It looks like the change was to "PROGS" ?
>
>>      disconnected-tracing.
>>      * gdb.trace/disconnected-tracing.c: New file.
>>      * gdb.trace/disconnected-tracing.exp: New file.
>>
>> On 03/29/12 02:51, Tom Tromey wrote:
>>>>>>>> ">" == Hui Zhu<hui_zhu@mentor.com>   writes:
>>>
>>>>> +gdb_test "set confirm off" ".*"
>>>
>>> I think we use gdb_test_no_output for things like this now.
>>> I'm not sure whether it applies to other cases in your patch.
>>>
>>>>> +gdb_test "delete" ".*"
>>>
>>> Perhaps just calling delete_breakpoints would work?
>>>
>>> Tom
>>
>> disconnected-tracing.txt
>>
>>
>> ---
>>   testsuite/gdb.trace/Makefile.in              |    2
>>   testsuite/gdb.trace/disconnected-tracing.c   |    5 ++
>>   testsuite/gdb.trace/disconnected-tracing.exp |   59 +++++++++++++++++++++++++++
>>   3 files changed, 65 insertions(+), 1 deletion(-)
>>
>> --- a/testsuite/gdb.trace/Makefile.in
>> +++ b/testsuite/gdb.trace/Makefile.in
>> @@ -5,7 +5,7 @@ srcdir = @srcdir@
>>
>>   PROGS = ax backtrace deltrace infotrace packetlen passc-dyn passcount \
>>   	report save-trace tfile tfind tracecmd tsv unavailable while-dyn \
>> -	while-stepping
>> +	while-stepping disconnected-tracing
>>
>>   all info install-info dvi install uninstall installcheck check:
>>   	@echo "Nothing to be done for $@..."
>> --- /dev/null
>> +++ b/testsuite/gdb.trace/disconnected-tracing.c
>> @@ -0,0 +1,5 @@
>> +int
>> +main()
>> +{
>> +  return 0;
>> +}
>> \ No newline at end of file
>
>
> Please add one.
>
> Add a copyright header while at it.  Best be consistent and
> make that a rule, even if the file is actually void of
> copyrightable content for now.
>
>
>> --- /dev/null
>> +++ b/testsuite/gdb.trace/disconnected-tracing.exp
>> @@ -0,0 +1,59 @@
>> +#   Copyright 2012 Free Software Foundation, Inc.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see<http://www.gnu.org/licenses/>.
>> +
>> +# Test of disconnected-tracing.
>> +
>> +load_lib "trace-support.exp";
>> +
>> +set testfile "disconnected-tracing"
>> +set executable $testfile
>> +set srcfile ${testfile}.c
>> +set binfile $objdir/$subdir/$testfile
>> +set expfile $testfile.exp
>> +set gdbserver_reconnect_p 1
>
>
> Please see gdb.base/solib-disc.exp, the only current user
> of gdbserver_reconnect_p.  Add:
>
> if { [info proc gdb_reconnect] == "" } {
>      return 0
> }
>
>> +
>> +if [prepare_for_testing $expfile $executable $srcfile \
>> +        {debug nowarnings}] {
>> +    untested "failed to prepare for trace tests"
>> +    return -1
>> +}
>> +
>> +if ![runto_main] {
>> +    fail "Can't run to main to check for trace support"
>> +    return -1
>> +}
>> +
>> +if ![gdb_target_supports_trace] {
>> +    unsupported "target does not support trace"
>> +    return -1;
>> +}
>> +
>> +gdb_test_no_output "set confirm off" ".*"
>
>
> Please always look at the resulting messages in gdb.sum:
>
> PASS: gdb.trace/disconnected-tracing.exp: .*
>                                            ^^
> PASS: gdb.trace/disconnected-tracing.exp: trace main
> PASS: gdb.trace/disconnected-tracing.exp: tstart
> PASS: gdb.trace/disconnected-tracing.exp: First info tracepoints
> PASS: gdb.trace/disconnected-tracing.exp: disconnect
> PASS: gdb.trace/disconnected-tracing.exp: Second info tracepoints
> PASS: gdb.trace/disconnected-tracing.exp: disconnect
> PASS: gdb.trace/disconnected-tracing.exp: Third info tracepoints
> PASS: gdb.trace/disconnected-tracing.exp: Fourth info tracepoints
>
> There are duplicate messages here.  See
> <http://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique>.
>
> I'd lowercase the "First", etc., in the test messages.  It's much more common.
>
>> +
>> +gdb_test "trace main" ".*"
>> +gdb_test "tstart" ".*"
>
>
> Use gdb_test_no_output for tstart:
>
>   gdb_test_no_output "tstart" "start trace experiment"
>
>> +
>> +gdb_test "info tracepoints" ".*in main at.*" "First info tracepoints"
>> +gdb_test "disconnect" ".*"
>> +
>> +gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
>
>
> Please see gdb.base/solib-disc.exp for how to reconnect to the target.
>
>> +gdb_test "info tracepoints" ".*in main at.*" "Second info tracepoints"
>> +gdb_test "disconnect" ".*"
>> +
>> +delete_breakpoints
>> +gdb_test "info tracepoints" ".*No tracepoints..*" "Third info tracepoints"
>> +
>> +gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
>
>> +gdb_test "info tracepoints" ".*in main at.*" "Fourth info tracepoints"
>
>
> But most importantly, I've tried this patch against a tree from just before
> the fixes, but the new tests all run cleanly there too.
>
> Also, should there be a "set disconnected-tracing on" somewhere?
> If not, then the test's filename appears misleading.  Maybe something
> with "reconnect" in the name would be more representative?
>

Oops, I forgot it.

I post a new version patch according to your comments.
And I have test it with gdb-7.4 and got fail.

Best,
Hui

2012-04-05  Hui Zhu  <hui_zhu@mentor.com>

	* gdb.trace/Makefile.in (PROGS): Add
	disconnected-tracing.
	* gdb.trace/disconnected-tracing.c: New file.
	* gdb.trace/disconnected-tracing.exp: New file.