This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Add breakpoint_created observer to update tracepoint_count.
On 11/09/2012 01:09 AM, Yao Qi wrote:
> On 11/08/2012 02:13 AM, Pedro Alves wrote:
>> > No need to install an observer for a notification that is emitted in the same
>> > module the new observer is in. This is internals of the breakpoints module.
>> > All set_breakpoint_count's calls are centralized in install_breakpoint, through
>> > set_breakpoint_number. All but break-range's, that is. I don't recall why
>> > that doesn't use install_breakpoint. Maybe it should.
>> >
> My original thought is to move tracepoint-related code from
> breakpoint.c to tracepoint.c, so register a breakpoint_created observer
> in tracepoint.c to update 'tracepoint_count'. Unfortunately,
> tracepoint commands use some breakpoint internal macros that prevent
> moving them to tracepoint.c, so I move the observer register code back
> to breakpoint.c.
I see.
>
> Yes, it is not necessary to register a observer to update some state in
> the same module.
>
>> > We should be able to put 'if (is_tracepoint) set_tracepoint_count()' in
>> > install_breakpoint too.
> Hmmm, that sounds the right place to update 'tracepoint_count'. How
> about patch below? A new test is added, without this fix, this test
> fails.
Looks good. Further comments below.
> 2012-11-09 Yao Qi <yao@codesourcery.com>
>
> * gdb.mi/mi-break.exp (test_abreak_creation): New.
I suggest:
* gdb.mi/mi-break.exp (test_abreak_creation): New procedure.
(top level): Call it.
And here ...
> +proc test_abreak_creation {} {
> + mi_gdb_test "522-break-insert -a main" \
> + "522\\^done,bkpt=\{number=\"10\",type=\"tracepoint\".*\"\}" \
> + "break-insert -a operation"
> +
> + mi_gdb_test "p \$tpnum" ".* = 10.*" "print \$tpnum"
> +}
> +
... I suggest checking tpnum before creating the tracepoint too
BTW, do we need "10.*", or is "10" good enough? "10.*" matches 100
or other bogus numbers too. And usually ".*" at the start of the
regex is not necessary.
So all in all (untested):
mi_gdb_test "p \$tpnum" " = void" "\$tpnum before tracepoint"
mi_gdb_test "522-break-insert -a main" \
"522\\^done,bkpt=\{number=\"10\",type=\"tracepoint\".*\"\}" \
"break-insert -a operation"
mi_gdb_test "p \$tpnum" " = 10" "\$tpnum after tracepoint"