This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] fix PR-15501
- From: Pedro Alves <palves at redhat dot com>
- To: "Waqas, Muhammad" <Muhammad_Waqas at mentor dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Thu, 22 Aug 2013 13:02:06 +0100
- Subject: Re: [PATCH] fix PR-15501
- References: <520A0453 dot 4070309 at codesourcery dot com> <520A6EEB dot 8010808 at redhat dot com> <520CAE24 dot 7050301 at codesourcery dot com>,<52153C74 dot 7080708 at redhat dot com> <53A5AC689E2AD547AE0EA5642E101306BB6120 at EU-MBX-04 dot mgc dot mentorg dot com>
On 08/22/2013 10:36 AM, Waqas, Muhammad wrote:
> +# This proc will be able to perform test for disable/enable
> +# commads on multiple locations and breakpoints,
> +# if args will be according to below explained values.
> +# Here arg are
> +# "what" is command (disable/enable),
> +# "what_res" is for breakpoints should be enabled or not,
> +# "p1/p2" are proc(pass/fail) but must be opposite.
> +#
> +# Here arg's values
> +# If "what" = "disable" then
> +# "what_res" = "n"
> +# "p1" = "pass"
> +# "p2" = "fail".
> +#
> +# If "what" = "enable" then
> +# "what_res" = "y"
> +# "p1" = "fail"
> +# "p2" = "pass".
Thanks. I suggest this edit then:
# Perform tests for disable/enable commands on multiple
# locations and breakpoints.
#
# WHAT - the command to test (disable/enable),
# WHAT_RES - whether breakpoints are expected to end
# up enabled or disabled.
# P1/P2 - proc to call (pass/fail). Must be
# opposites.
#
# Furthermore, arguments must follow these rules:
#
# If "what" = "disable" then
# "what_res" = "n"
# "p1" = "pass"
# "p2" = "fail".
#
# If "what" = "enable" then
# "what_res" = "y"
# "p1" = "fail"
# "p2" = "pass".
#
> +
> +proc test_ena_dis_br { what what_res p1 p2 } {
> + global b1
> + global b2
> + global b3
> + global b4
> + global gdb_prompt
> +
> + # Now enable/disable $b.1 $b2.1.
> + gdb_test_no_output "$what $b1.1 $b2.1" "$what \$b1.1 \$b2.1"
> + set test1 "${what}d \$b1.1 and \$b2.1"
> +
> + # Now $b1.1 and $b2.1 should be enabled/disabled
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b1}.1)(\[^\n\r\]*)( n.*)(${b2}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re ".*$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +
> + # Now enable/disbale $b1 fooo.1, it should give error on fooo.
Typo: disbale.
> + # Now enable/disbale $b1 fooo.1, it should give error on fooo.
> + gdb_test "$what $b1 fooo.1" \
> + "Bad breakpoint number 'fooo'" \
> + "$what \$b1 fooo.1"
> +
> + # $b1 should be enabled/disabled.
> + gdb_test "info break" \
> + "(${b1})(\[^\n\r]*)( $what_res.*)" \
> + "${what}d \$b1"
> +
> + # Here "oppos" is commmand that should be opposite of "what".
> + set oppos "enable"
> + set oppos_res "y"
> +
> + if { $what == "enable" } {
> + set oppos "disable"
> + set oppos_res "n"
Indentation is wrong here. If we're doing this, then
one has to wonder why does the routine expect all
these arguments:
# If "what" = "disable" then
# "what_res" = "n"
# "p1" = "pass"
# "p2" = "fail".
#
# If "what" = "enable" then
# "what_res" = "y"
# "p1" = "fail"
# "p2" = "pass".
... instead of accepting only "what", and computing
the rest from that itself.
> +
Spurious newline.
> + }
> +
> + gdb_test_no_output "$oppos $b3" "$oppos \$b3"
> + gdb_test_no_output "$what $b4 $b3.1" "$what \$b4 \$b3.1"
> + set test1 "${what}d \$b4 and \$b3.1,remain ${oppos}d \$b3"
> +
> + # Now $b4 $b3.1 should be enabled/disabled and
> + # $b3 should remain enabled/disabled
enabled/disabled ... enabled/disabled
That's confusing, and says practically nothing... They're
all either enabled or disabled. I suggest adjusting all
these comments to be written in terms of "enabled",
with the disabled state written on the right, in parens.
So that'd be:
# Now $b4 $b3.1 should be enabled(disabled) and
# $b3 should remain disabled(enabled).
So, when reading this, one now clearly understands
that $b3 should end up in the opposite state of$b4 $b3.1
(it that's what should happen).
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b3}.1)(\[^\n\r\]*)( n.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re "(${b3})(\[^\n\r]*)( $oppos_res.*)(${b4})(\[^\n\r\]*)( $what_res.*)$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +
> + # Now enable/disable $b4.1 fooobaar and
> + # it should give warning on fooobaar.
> + gdb_test "$what $b4.1 fooobaar" \
> + "warning: bad breakpoint number at or near 'fooobaar'" \
> + "$what \$b4.1 fooobar"
> + set test1 "${what}d \$b4.1"
> +
> + # $b4.1 should be enabled/disbaled
Typo. Please remember to always self review for typos. I
think I'm noticing them at all iterations.
> + gdb_test_multiple "info break" "$test1" {
> + -re "(${b4}.1)(\[^\n\r\]*)( n.*)$gdb_prompt $" {
> + $p1 "$test1"
> + }
> + -re ".*$gdb_prompt $" {
> + $p2 "$test1"
> + }
> + }
> +}
> +
> +test_ena_dis_br "disable" "n" "pass" "fail"
> +test_ena_dis_br "enable" "y" "fail" "pass"
> +
> gdb_exit
> return 0
Thanks,
--
Pedro Alves