* [RFA] info break/watch/trace use get_number_or_range, take two
@ 2011-02-21 23:00 Michael Snyder
2011-02-22 9:27 ` Pedro Alves
2011-02-22 11:49 ` Eli Zaretskii
0 siblings, 2 replies; 10+ messages in thread
From: Michael Snyder @ 2011-02-21 23:00 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 126 bytes --]
Supercedes previous submission.
Fixed Pedro's issues. Tested with maint info break.
Regression tested on RHEL5, x86_64.
[-- Attachment #2: infobreak.txt --]
[-- Type: text/plain, Size: 20545 bytes --]
2011-02-21 Michael Snyder <msnyder@vmware.com>
* breakpoint.c (breakpoint_1): Change first argument from an int
to a char pointer, so that the function now accepts a list of
breakpoints rather than just one. Use new function
'number_is_in_list' to implement.
(breakpoints_info): Pass char * instead of int to breakpoint_1.
(watchpoints_info): Ditto.
(tracepoints_info): Ditto.
(maintenance_info_breakpoints): Ditto.
(_initialize_breakpoint): Update help strings to reflect the fact
that these functions can now take more than one argument.
* cli/cli-utils.c (number_is_in_list): New function.
* cli/cli-utils.h (number_is_in_list): Export.
2011-02-21 Michael Snyder <msnyder@vmware.com>
* gdb.texinfo (Set Breaks): Add @dots{} to arguments of info break.
(Set Watchpoints): Add @dots{} to argument of info watchpoints.
(Listing Tracepoints): Add @dots{} to argument of info tracepoints.
2011-02-21 Michael Snyder <msnyder@vmware.com>
* gdb.base/break.exp: Add tests for "info break" with arguments.
* gdb.trace/infotrace.exp: Update patterns for error and help.
* gdb.base/completion.exp: Update pattern.
* gdb.base/ena-dis-br.exp: Update pattern.
* gdb.base/help.exp: Update patterns.
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.540
diff -u -p -u -p -r1.540 breakpoint.c
--- breakpoint.c 21 Feb 2011 18:40:08 -0000 1.540
+++ breakpoint.c 21 Feb 2011 20:47:06 -0000
@@ -133,7 +133,8 @@ static void breakpoints_info (char *, in
static void watchpoints_info (char *, int);
-static int breakpoint_1 (int, int, int (*) (const struct breakpoint *));
+static int breakpoint_1 (char *, int,
+ int (*) (const struct breakpoint *));
static int breakpoint_cond_eval (void *);
@@ -5102,7 +5103,7 @@ user_breakpoint_p (struct breakpoint *b)
breakpoints listed. */
static int
-breakpoint_1 (int bnum, int allflag,
+breakpoint_1 (char *args, int allflag,
int (*filter) (const struct breakpoint *))
{
struct breakpoint *b;
@@ -5119,28 +5120,36 @@ breakpoint_1 (int bnum, int allflag,
required for address fields. */
nr_printable_breakpoints = 0;
ALL_BREAKPOINTS (b)
- if (bnum == -1
- || bnum == b->number)
- {
- /* If we have a filter, only list the breakpoints it accepts. */
- if (filter && !filter (b))
- continue;
-
- if (allflag || user_breakpoint_p (b))
- {
- int addr_bit, type_len;
+ {
+ /* If we have a filter, only list the breakpoints it accepts. */
+ if (filter && !filter (b))
+ continue;
+
+ /* If we have an "args" string, it is a list of breakpoints to
+ accept. Skip the others. */
+ if (args != NULL && *args != '\0')
+ {
+ if (allflag && (parse_and_eval_long (args) != b->number))
+ continue;
+ if (!allflag && !number_is_in_list (args, b->number))
+ continue;
+ }
- addr_bit = breakpoint_address_bits (b);
- if (addr_bit > print_address_bits)
- print_address_bits = addr_bit;
+ if (allflag || user_breakpoint_p (b))
+ {
+ int addr_bit, type_len;
- type_len = strlen (bptype_string (b->type));
- if (type_len > print_type_col_width)
- print_type_col_width = type_len;
+ addr_bit = breakpoint_address_bits (b);
+ if (addr_bit > print_address_bits)
+ print_address_bits = addr_bit;
- nr_printable_breakpoints++;
- }
- }
+ type_len = strlen (bptype_string (b->type));
+ if (type_len > print_type_col_width)
+ print_type_col_width = type_len;
+
+ nr_printable_breakpoints++;
+ }
+ }
if (opts.addressprint)
bkpttbl_chain
@@ -5169,16 +5178,16 @@ breakpoint_1 (int bnum, int allflag,
annotate_field (3);
ui_out_table_header (uiout, 3, ui_left, "enabled", "Enb"); /* 4 */
if (opts.addressprint)
- {
- if (nr_printable_breakpoints > 0)
- annotate_field (4);
- if (print_address_bits <= 32)
- ui_out_table_header (uiout, 10, ui_left,
- "addr", "Address"); /* 5 */
- else
- ui_out_table_header (uiout, 18, ui_left,
- "addr", "Address"); /* 5 */
- }
+ {
+ if (nr_printable_breakpoints > 0)
+ annotate_field (4);
+ if (print_address_bits <= 32)
+ ui_out_table_header (uiout, 10, ui_left,
+ "addr", "Address"); /* 5 */
+ else
+ ui_out_table_header (uiout, 18, ui_left,
+ "addr", "Address"); /* 5 */
+ }
if (nr_printable_breakpoints > 0)
annotate_field (5);
ui_out_table_header (uiout, 40, ui_noalign, "what", "What"); /* 6 */
@@ -5187,22 +5196,34 @@ breakpoint_1 (int bnum, int allflag,
annotate_breakpoints_table ();
ALL_BREAKPOINTS (b)
- {
- QUIT;
- if (bnum == -1
- || bnum == b->number)
- {
- /* If we have a filter, only list the breakpoints it accepts. */
- if (filter && !filter (b))
- continue;
-
- /* We only print out user settable breakpoints unless the
- allflag is set. */
- if (allflag || user_breakpoint_p (b))
- print_one_breakpoint (b, &last_loc, print_address_bits, allflag);
- }
- }
-
+ {
+ QUIT;
+ /* If we have a filter, only list the breakpoints it accepts. */
+ if (filter && !filter (b))
+ continue;
+
+ /* If we have an "args" string, it is a list of breakpoints to
+ accept. Skip the others. */
+
+ if (args != NULL && *args != '\0')
+ {
+ if (allflag) /* maintenance info breakpoint */
+ {
+ if (parse_and_eval_long (args) != b->number)
+ continue;
+ }
+ else /* all others */
+ {
+ if (!number_is_in_list (args, b->number))
+ continue;
+ }
+ }
+ /* We only print out user settable breakpoints unless the
+ allflag is set. */
+ if (allflag || user_breakpoint_p (b))
+ print_one_breakpoint (b, &last_loc, print_address_bits, allflag);
+ }
+
do_cleanups (bkpttbl_chain);
if (nr_printable_breakpoints == 0)
@@ -5211,12 +5232,12 @@ breakpoint_1 (int bnum, int allflag,
empty list. */
if (!filter)
{
- if (bnum == -1)
+ if (args == NULL || *args == '\0')
ui_out_message (uiout, 0, "No breakpoints or watchpoints.\n");
else
ui_out_message (uiout, 0,
- "No breakpoint or watchpoint number %d.\n",
- bnum);
+ "No breakpoint or watchpoint matching '%s'.\n",
+ args);
}
}
else
@@ -5252,46 +5273,31 @@ default_collect_info (void)
}
static void
-breakpoints_info (char *bnum_exp, int from_tty)
+breakpoints_info (char *args, int from_tty)
{
- int bnum = -1;
-
- if (bnum_exp)
- bnum = parse_and_eval_long (bnum_exp);
-
- breakpoint_1 (bnum, 0, NULL);
+ breakpoint_1 (args, 0, NULL);
default_collect_info ();
}
static void
-watchpoints_info (char *wpnum_exp, int from_tty)
+watchpoints_info (char *args, int from_tty)
{
- int wpnum = -1, num_printed;
-
- if (wpnum_exp)
- wpnum = parse_and_eval_long (wpnum_exp);
-
- num_printed = breakpoint_1 (wpnum, 0, is_watchpoint);
+ int num_printed = breakpoint_1 (args, 0, is_watchpoint);
if (num_printed == 0)
{
- if (wpnum == -1)
+ if (args == NULL || *args == '\0')
ui_out_message (uiout, 0, "No watchpoints.\n");
else
- ui_out_message (uiout, 0, "No watchpoint number %d.\n", wpnum);
+ ui_out_message (uiout, 0, "No watchpoint matching '%s'.\n", args);
}
}
static void
-maintenance_info_breakpoints (char *bnum_exp, int from_tty)
+maintenance_info_breakpoints (char *args, int from_tty)
{
- int bnum = -1;
-
- if (bnum_exp)
- bnum = parse_and_eval_long (bnum_exp);
-
- breakpoint_1 (bnum, 1, NULL);
+ breakpoint_1 (args, 1, NULL);
default_collect_info ();
}
@@ -11510,21 +11516,18 @@ create_tracepoint_from_upload (struct up
omitted. */
static void
-tracepoints_info (char *tpnum_exp, int from_tty)
+tracepoints_info (char *args, int from_tty)
{
- int tpnum = -1, num_printed;
-
- if (tpnum_exp)
- tpnum = parse_and_eval_long (tpnum_exp);
+ int num_printed;
- num_printed = breakpoint_1 (tpnum, 0, is_tracepoint);
+ num_printed = breakpoint_1 (args, 0, is_tracepoint);
if (num_printed == 0)
{
- if (tpnum == -1)
+ if (args == NULL || *args == '\0')
ui_out_message (uiout, 0, "No tracepoints.\n");
else
- ui_out_message (uiout, 0, "No tracepoint number %d.\n", tpnum);
+ ui_out_message (uiout, 0, "No tracepoint matching '%s'.\n", args);
}
default_collect_info ();
@@ -12214,7 +12217,7 @@ breakpoint set."));
}
add_info ("breakpoints", breakpoints_info, _("\
-Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
+Status of user-settable breakpoints listed, or all breakpoints if no argument.\n\
The \"Type\" column indicates one of:\n\
\tbreakpoint - normal breakpoint\n\
\twatchpoint - watchpoint\n\
@@ -12362,9 +12365,7 @@ the memory to which it refers."));
set_cmd_completer (c, expression_completer);
add_info ("watchpoints", watchpoints_info, _("\
-Status of watchpoints, or watchpoint number NUMBER."));
-
-
+Status of watchpoints listed, or all watchpoints if no argument."));
/* XXX: cagney/2005-02-23: This should be a boolean, and should
respond to changes - contrary to the description. */
@@ -12430,7 +12431,7 @@ Do \"help tracepoints\" for info on othe
set_cmd_completer (c, location_completer);
add_info ("tracepoints", tracepoints_info, _("\
-Status of tracepoints, or tracepoint number NUMBER.\n\
+Status of tracepoints listed, or all tracepoints if no argument.\n\
Convenience variable \"$tpnum\" contains the number of the\n\
last tracepoint set."));
Index: cli/cli-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-utils.c,v
retrieving revision 1.8
diff -u -p -u -p -r1.8 cli-utils.c
--- cli/cli-utils.c 21 Feb 2011 18:13:17 -0000 1.8
+++ cli/cli-utils.c 21 Feb 2011 20:47:06 -0000
@@ -161,6 +161,27 @@ get_number_or_range (char **pp)
return last_retval;
}
+/* Accept a number and a string-form list of numbers such as is
+ accepted by get_number_or_range. Return TRUE if the number is
+ in the list.
+
+ By definition, an empty list includes all numbers. This is to
+ be interpreted as typing a command such as "delete break" with
+ no arguments. */
+
+int
+number_is_in_list (char *list, int number)
+{
+ if (list == NULL || *list == '\0')
+ return 1;
+
+ while (list != NULL && *list != '\0')
+ if (get_number_or_range (&list) == number)
+ return 1;
+
+ return 0;
+}
+
/* See documentation in cli-utils.h. */
char *
Index cli/cli-utils.h
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-utils.h,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 cli-utils.h
--- cli/cli-utils.h 21 Feb 2011 18:13:17 -0000 1.9
+++ cli/cli-utils.h 21 Feb 2011 20:47:06 -0000
@@ -45,6 +45,16 @@ extern int get_number (char **);
extern int get_number_or_range (char **);
+/* Accept a number and a string-form list of numbers such as is
+ accepted by get_number_or_range. Return TRUE if the number is
+ in the list.
+
+ By definition, an empty list includes all numbers. This is to
+ be interpreted as typing a command such as "delete break" with
+ no arguments. */
+
+extern int number_is_in_list (char *list, int number);
+
/* Skip leading whitespace characters in INP, returning an updated
pointer. If INP is NULL, return NULL. */
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.801
diff -u -p -u -p -r1.801 gdb.texinfo
--- doc/gdb.texinfo 21 Feb 2011 08:40:27 -0000 1.801
+++ doc/gdb.texinfo 21 Feb 2011 20:47:09 -0000
@@ -3438,12 +3438,12 @@ optionally be surrounded by spaces.
@kindex info breakpoints
@cindex @code{$_} and @code{info breakpoints}
-@item info breakpoints @r{[}@var{n}@r{]}
-@itemx info break @r{[}@var{n}@r{]}
+@item info breakpoints @r{[}@var{n}@dots{}@r{]}
+@itemx info break @r{[}@var{n}@dots{}@r{]}
Print a table of all breakpoints, watchpoints, and catchpoints set and
not deleted. Optional argument @var{n} means print information only
-about the specified breakpoint (or watchpoint or catchpoint). For
-each breakpoint, following columns are printed:
+about the specified breakpoint(s) (or watchpoint(s) or catchpoint(s)).
+For each breakpoint, following columns are printed:
@table @emph
@item Breakpoint Numbers
@@ -3763,8 +3763,8 @@ by the program.
Set a watchpoint that will break when @var{expr} is either read from
or written into by the program.
-@kindex info watchpoints @r{[}@var{n}@r{]}
-@item info watchpoints
+@kindex info watchpoints @r{[}@var{n}@dots{}@r{]}
+@item info watchpoints @r{[}@var{n}@dots{}@r{]}
This command prints a list of watchpoints, using the same format as
@code{info break} (@pxref{Set Breaks}).
@end table
@@ -10289,10 +10289,10 @@ tracepoint hit.
@subsection Listing Tracepoints
@table @code
-@kindex info tracepoints
-@kindex info tp
+@kindex info tracepoints @r{[}@var{n}@dots{}@r{]}
+@kindex info tp @r{[}@var{n}@dots{}@r{]}
@cindex information about tracepoints
-@item info tracepoints @r{[}@var{num}@r{]}
+@item info tracepoints @r{[}@var{num}@dots{}@r{]}
Display information about the tracepoint @var{num}. If you don't
specify a tracepoint number, displays information about all the
tracepoints defined so far. The format is similar to that used for
Index: testsuite/gdb.base/break.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/break.exp,v
retrieving revision 1.44
diff -u -p -u -p -r1.44 break.exp
--- testsuite/gdb.base/break.exp 1 Jan 2011 15:33:40 -0000 1.44
+++ testsuite/gdb.base/break.exp 21 Feb 2011 20:47:09 -0000
@@ -150,6 +150,94 @@ gdb_test "info break" \
\[0-9\]+\[\t \]+breakpoint keep y.* in multi_line_while_conditional at .*$srcfile:$bp_location4" \
"breakpoint info"
+#
+# Test info breakpoint with arguments
+#
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+set see6 0
+
+gdb_test_multiple "info break 2 4 6" "info break 2 4 6" {
+ -re "1\[\t \]+breakpoint *keep y\[^\r\n\]*:$main_line\[^\r\n\]*" {
+ set see1 1
+ exp_continue
+ }
+ -re "2\[\t \]+breakpoint *keep y\[^\r\n\]* in marker2 at \[^\r\n\]*" {
+ set see2 1
+ exp_continue
+ }
+ -re "3\[\t \]+breakpoint *keep y\[^\r\n\]*$bp_location7\[^\r\n\]*" {
+ set see3 1
+ exp_continue
+ }
+ -re "4\[\t \]+breakpoint *keep y\[^\r\n\]*$bp_location1\[^\r\n\]*" {
+ set see4 1
+ exp_continue
+ }
+ -re "5\[\t \]+breakpoint *keep y\[^\r\n\]*$bp_location1\[^\r\n\]*" {
+ set see5 1
+ exp_continue
+ }
+ -re "6\[\t \]+breakpoint *keep y\[^\r\n\]*$bp_location2\[^\r\n\]*" {
+ set see6 1
+ exp_continue
+ }
+ -re ".*$gdb_prompt $" {
+ if { !$see1 && $see2 && !$see3 && $see4 && !$see5 && $see6 } then {
+ pass "info break 2 4 6"
+ } else {
+ fail "info break 2 4 6"
+ }
+ }
+}
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+set see6 0
+
+gdb_test_multiple "info break 3-5" "info break 3-5" {
+ -re "1\[\t \]+breakpoint *keep y.* in main at .*:$main_line\[^\r\n\]*" {
+ set see1 1
+ exp_continue
+ }
+ -re "2\[\t \]+breakpoint *keep y\[^\r\n\]* in marker2 at \[^\r\n\]*" {
+ set see2 1
+ exp_continue
+ }
+ -re "3\[\t \]+breakpoint *keep y\[^\r\n\]*$bp_location7\[^\r\n\]*" {
+ set see3 1
+ exp_continue
+ }
+ -re "4\[\t \]+breakpoint *keep y\[^\r\n\]*$bp_location1\[^\r\n\]*" {
+ set see4 1
+ exp_continue
+ }
+ -re "5\[\t \]+breakpoint *keep y\[^\r\n\]*$bp_location1\[^\r\n\]*" {
+ set see5 1
+ exp_continue
+ }
+ -re "6\[\t \]+breakpoint *keep y\[^\r\n\]*$bp_location2\[^\r\n\]*" {
+ set see6 1
+ exp_continue
+ }
+ -re ".*$gdb_prompt $" {
+ if { !$see1 && !$see2 && $see3 && $see4 && $see5 && !$see6 } then {
+ pass "info break 3-5"
+ } else {
+ fail "info break 3-5"
+ }
+ }
+}
+
+gdb_test "print !$see1 && !$see2 && $see3 && $see4 && $see5 && !$see6" "" ""
+
# FIXME: The rest of this test doesn't work with anything that can't
# handle arguments.
# Huh? There doesn't *appear* to be anything that passes arguments
Index: testsuite/gdb.trace/infotrace.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.trace/infotrace.exp,v
retrieving revision 1.18
diff -u -p -u -p -r1.18 infotrace.exp
--- testsuite/gdb.trace/infotrace.exp 1 Jan 2011 15:33:50 -0000 1.18
+++ testsuite/gdb.trace/infotrace.exp 21 Feb 2011 20:47:09 -0000
@@ -73,7 +73,7 @@ gdb_test "info tracepoint $asm_test_num"
# 2.3 info tracepoint (invalid tracepoint number)
gdb_test "info tracepoint [expr $c_test_num + $asm_test_num]" \
- "No tracepoint number [expr $c_test_num + $asm_test_num]." \
+ "No tracepoint matching '[expr $c_test_num + $asm_test_num]'." \
"2.3: info tracepoint (invalid tracepoint number)"
# 2.4 info tracepoints (list of numbers)
@@ -89,6 +89,6 @@ gdb_test_multiple "info tracepoints $c_t
# 2.5 help info trace
gdb_test "help info tracepoints" \
- "Status of tracepoints, or tracepoint number NUMBER.*" \
+ "Status of tracepoints listed, or all tracepoints if no argument.*" \
"2.5: help info tracepoints"
Index: testsuite/gdb.base/completion.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/completion.exp,v
retrieving revision 1.49
diff -u -p -u -p -r1.49 completion.exp
--- testsuite/gdb.base/completion.exp 1 Jan 2011 15:33:41 -0000 1.49
+++ testsuite/gdb.base/completion.exp 21 Feb 2011 22:21:26 -0000
@@ -352,7 +352,7 @@ gdb_expect {
-re "^help info watchpoints $"\
{ send_gdb "\n"
gdb_expect {
- -re "Status of watchpoints, .*\r\n.*$gdb_prompt $"\
+ -re "Status of watchpoints listed.*\r\n.*$gdb_prompt $"\
{ pass "complete help info wat" }
-re ".*$gdb_prompt $" { fail "complete help info wat"}
timeout {fail "(timeout) complete help info wat"}
Index: testsuite/gdb.base/ena-dis-br.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ena-dis-br.exp,v
retrieving revision 1.15
diff -u -p -u -p -r1.15 ena-dis-br.exp
--- testsuite/gdb.base/ena-dis-br.exp 1 Jan 2011 15:33:41 -0000 1.15
+++ testsuite/gdb.base/ena-dis-br.exp 21 Feb 2011 22:21:37 -0000
@@ -154,7 +154,7 @@ gdb_test "continue" \
"continue to auto-deleted break marker3"
gdb_test "info break $bp" \
- ".*No breakpoint or watchpoint number.*" \
+ ".*No breakpoint or watchpoint matching.*" \
"info auto-deleted break marker3"
# Verify that we can set a breakpoint and manually disable it (we've
Index: testsuite/gdb.base/help.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/help.exp,v
retrieving revision 1.49
diff -u -p -u -p -r1.49 help.exp
--- testsuite/gdb.base/help.exp 15 Feb 2011 21:17:53 -0000 1.49
+++ testsuite/gdb.base/help.exp 21 Feb 2011 22:21:43 -0000
@@ -238,7 +238,9 @@ gdb_test "help info all-registers" "List
# test help info args
gdb_test "help info args" "Argument variables of current stack frame\." "help info args"
# test help info breakpoints
-gdb_test "help info breakpoints" "Status of user-settable breakpoints, or breakpoint number NUMBER\..*\[\r\n\]+breakpoint set\." "help info breakpoints"
+gdb_test "help info breakpoints" \
+ "Status of user-settable breakpoints listed, or all breakpoints if no argument\..*\[\r\n\]+breakpoint set\." \
+ "help info breakpoints"
# test help info catch
gdb_test "help info catch" "Exceptions that can be caught in the current stack frame\." "help info catch"
# test help info copying
@@ -290,7 +292,9 @@ gdb_test "help info variables" "All glob
# test help info warranty
gdb_test "help info warranty" "Various kinds of warranty you do not have\." "help info warranty"
# test help info watchpoints
-gdb_test "help info watchpoints" "Status of watchpoints, or watchpoint number NUMBER\." "help info watchpoints"
+gdb_test "help info watchpoints" \
+ "Status of watchpoints listed, or all watchpoints if no argument\." \
+ "help info watchpoints"
# test help inspect
gdb_test "help inspect" "Same as \"print\" command, except that if you are running in the epoch\[\r\n\]+environment, the value is printed in its own window\." "help inspect"
# test help jump
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFA] info break/watch/trace use get_number_or_range, take two
2011-02-21 23:00 [RFA] info break/watch/trace use get_number_or_range, take two Michael Snyder
@ 2011-02-22 9:27 ` Pedro Alves
2011-02-22 10:56 ` Eli Zaretskii
2011-02-22 18:31 ` Michael Snyder
2011-02-22 11:49 ` Eli Zaretskii
1 sibling, 2 replies; 10+ messages in thread
From: Pedro Alves @ 2011-02-22 9:27 UTC (permalink / raw)
To: gdb-patches; +Cc: Michael Snyder
On Monday 21 February 2011 22:23:28, Michael Snyder wrote:
> Supercedes previous submission.
>
> Fixed Pedro's issues. Tested with maint info break.
"Pedro's issues"? Eh, I have no personal issues!
Thanks, I like this much better.
> + /* If we have an "args" string, it is a list of breakpoints to
> + accept. Skip the others. */
> + if (args != NULL && *args != '\0')
> + {
> + if (allflag && (parse_and_eval_long (args) != b->number))
Spurious parens?
> + continue;
> + if (!allflag && !number_is_in_list (args, b->number))
> + continue;
> + }
I think that parse_and_eval_long means that
"maint info break 2-3" will print breakpoint -1,
while
"info break 2-3" will print breakpoints 2 and 3
It's fine with me, just pointing it out.
> - addr_bit = breakpoint_address_bits (b);
> - if (addr_bit > print_address_bits)
> - print_address_bits = addr_bit;
> + if (allflag || user_breakpoint_p (b))
> + {
> + int addr_bit, type_len;
>
> - type_len = strlen (bptype_string (b->type));
> - if (type_len > print_type_col_width)
> - print_type_col_width = type_len;
> + addr_bit = breakpoint_address_bits (b);
> + if (addr_bit > print_address_bits)
> + print_address_bits = addr_bit;
>
> - nr_printable_breakpoints++;
> - }
> - }
> + type_len = strlen (bptype_string (b->type));
> + if (type_len > print_type_col_width)
> + print_type_col_width = type_len;
> +
> + nr_printable_breakpoints++;
> + }
> + }
>
> if (opts.addressprint)
> bkpttbl_chain
> @@ -5169,16 +5178,16 @@ breakpoint_1 (int bnum, int allflag,
> annotate_field (3);
> ui_out_table_header (uiout, 3, ui_left, "enabled", "Enb"); /* 4 */
> if (opts.addressprint)
> - {
> - if (nr_printable_breakpoints > 0)
> - annotate_field (4);
> - if (print_address_bits <= 32)
> - ui_out_table_header (uiout, 10, ui_left,
> - "addr", "Address"); /* 5 */
> - else
> - ui_out_table_header (uiout, 18, ui_left,
> - "addr", "Address"); /* 5 */
> - }
> + {
> + if (nr_printable_breakpoints > 0)
> + annotate_field (4);
> + if (print_address_bits <= 32)
> + ui_out_table_header (uiout, 10, ui_left,
> + "addr", "Address"); /* 5 */
> + else
> + ui_out_table_header (uiout, 18, ui_left,
> + "addr", "Address"); /* 5 */
> + }
Please let's keep big formatting changes separate from
functional changes. We ask that in patch submissions, so we
should follow the same practice ourselves. It's really a good
rule BTW. I think of that as one case of what I call it
The Principle of Least Reversibility (TM), so explained:
If by chance this patch needs to be reverted, you don't want
to revert the whitespace fixes. Hence, you'll want to make
the whitespace fixes a separate patch. The same rule applies
to preparatory fixes and changes, and code moves (where if
you move the code, you should NOT change it in any other way).
> add_info ("breakpoints", breakpoints_info, _("\
> -Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
> +Status of user-settable breakpoints listed, or all breakpoints if no argument.\n\
"listed" doesn't sound obviously referring to the spec
you pass as argument to the command. "listed where? the
command itself is printing a list." was my thought. Is
there any other way to spell that?
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFA] info break/watch/trace use get_number_or_range, take two
2011-02-22 9:27 ` Pedro Alves
@ 2011-02-22 10:56 ` Eli Zaretskii
2011-02-22 18:32 ` Michael Snyder
2011-02-22 18:31 ` Michael Snyder
1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2011-02-22 10:56 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, msnyder
> From: Pedro Alves <pedro@codesourcery.com>
> Date: Tue, 22 Feb 2011 09:16:51 +0000
> Cc: Michael Snyder <msnyder@vmware.com>
>
> > add_info ("breakpoints", breakpoints_info, _("\
> > -Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
> > +Status of user-settable breakpoints listed, or all breakpoints if no argument.\n\
>
> "listed" doesn't sound obviously referring to the spec
> you pass as argument to the command. "listed where? the
> command itself is printing a list." was my thought. Is
> there any other way to spell that?
How about just losing the "listed" part? What important information
does it convey in this context?
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFA] info break/watch/trace use get_number_or_range, take two
2011-02-22 10:56 ` Eli Zaretskii
@ 2011-02-22 18:32 ` Michael Snyder
2011-02-22 18:48 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Michael Snyder @ 2011-02-22 18:32 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Pedro Alves, gdb-patches
Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@codesourcery.com>
>> Date: Tue, 22 Feb 2011 09:16:51 +0000
>> Cc: Michael Snyder <msnyder@vmware.com>
>>
>>> add_info ("breakpoints", breakpoints_info, _("\
>>> -Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
>>> +Status of user-settable breakpoints listed, or all breakpoints if no argument.\n\
>> "listed" doesn't sound obviously referring to the spec
>> you pass as argument to the command. "listed where? the
>> command itself is printing a list." was my thought. Is
>> there any other way to spell that?
>
> How about just losing the "listed" part? What important information
> does it convey in this context?
What wording do you suggest?
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFA] info break/watch/trace use get_number_or_range, take two
2011-02-22 18:32 ` Michael Snyder
@ 2011-02-22 18:48 ` Eli Zaretskii
2011-02-22 19:02 ` Michael Snyder
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2011-02-22 18:48 UTC (permalink / raw)
To: Michael Snyder; +Cc: pedro, gdb-patches
> Date: Tue, 22 Feb 2011 10:31:47 -0800
> From: Michael Snyder <msnyder@vmware.com>
> CC: Pedro Alves <pedro@codesourcery.com>,
> "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>
> Eli Zaretskii wrote:
> >> From: Pedro Alves <pedro@codesourcery.com>
> >> Date: Tue, 22 Feb 2011 09:16:51 +0000
> >> Cc: Michael Snyder <msnyder@vmware.com>
> >>
> >>> add_info ("breakpoints", breakpoints_info, _("\
> >>> -Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
> >>> +Status of user-settable breakpoints listed, or all breakpoints if no argument.\n\
> >> "listed" doesn't sound obviously referring to the spec
> >> you pass as argument to the command. "listed where? the
> >> command itself is printing a list." was my thought. Is
> >> there any other way to spell that?
> >
> > How about just losing the "listed" part? What important information
> > does it convey in this context?
>
> What wording do you suggest?
How about
Status of specified breakpoints (all user-settable breakpoints if no argument).
?
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFA] info break/watch/trace use get_number_or_range, take two
2011-02-22 18:48 ` Eli Zaretskii
@ 2011-02-22 19:02 ` Michael Snyder
0 siblings, 0 replies; 10+ messages in thread
From: Michael Snyder @ 2011-02-22 19:02 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: pedro, gdb-patches
Eli Zaretskii wrote:
>> Date: Tue, 22 Feb 2011 10:31:47 -0800
>> From: Michael Snyder <msnyder@vmware.com>
>> CC: Pedro Alves <pedro@codesourcery.com>,
>> "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>>
>> Eli Zaretskii wrote:
>>>> From: Pedro Alves <pedro@codesourcery.com>
>>>> Date: Tue, 22 Feb 2011 09:16:51 +0000
>>>> Cc: Michael Snyder <msnyder@vmware.com>
>>>>
>>>>> add_info ("breakpoints", breakpoints_info, _("\
>>>>> -Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
>>>>> +Status of user-settable breakpoints listed, or all breakpoints if no argument.\n\
>>>> "listed" doesn't sound obviously referring to the spec
>>>> you pass as argument to the command. "listed where? the
>>>> command itself is printing a list." was my thought. Is
>>>> there any other way to spell that?
>>> How about just losing the "listed" part? What important information
>>> does it convey in this context?
>> What wording do you suggest?
>
> How about
>
> Status of specified breakpoints (all user-settable breakpoints if no argument).
>
> ?
It's a deal.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA] info break/watch/trace use get_number_or_range, take two
2011-02-22 9:27 ` Pedro Alves
2011-02-22 10:56 ` Eli Zaretskii
@ 2011-02-22 18:31 ` Michael Snyder
1 sibling, 0 replies; 10+ messages in thread
From: Michael Snyder @ 2011-02-22 18:31 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves wrote:
> On Monday 21 February 2011 22:23:28, Michael Snyder wrote:
>> Supercedes previous submission.
>>
>> Fixed Pedro's issues. Tested with maint info break.
>
> "Pedro's issues"? Eh, I have no personal issues!
>
> Thanks, I like this much better.
>
>> + /* If we have an "args" string, it is a list of breakpoints to
>> + accept. Skip the others. */
>> + if (args != NULL && *args != '\0')
>> + {
>> + if (allflag && (parse_and_eval_long (args) != b->number))
>
> Spurious parens?
OK
>> + continue;
>> + if (!allflag && !number_is_in_list (args, b->number))
>> + continue;
>> + }
>
> I think that parse_and_eval_long means that
>
> "maint info break 2-3" will print breakpoint -1,
>
> while
>
> "info break 2-3" will print breakpoints 2 and 3
>
>
> It's fine with me, just pointing it out.
Just preserving the existing behavior.
>> - addr_bit = breakpoint_address_bits (b);
>> - if (addr_bit > print_address_bits)
>> - print_address_bits = addr_bit;
>> + if (allflag || user_breakpoint_p (b))
>> + {
>> + int addr_bit, type_len;
>>
>> - type_len = strlen (bptype_string (b->type));
>> - if (type_len > print_type_col_width)
>> - print_type_col_width = type_len;
>> + addr_bit = breakpoint_address_bits (b);
>> + if (addr_bit > print_address_bits)
>> + print_address_bits = addr_bit;
>>
>> - nr_printable_breakpoints++;
>> - }
>> - }
>> + type_len = strlen (bptype_string (b->type));
>> + if (type_len > print_type_col_width)
>> + print_type_col_width = type_len;
>> +
>> + nr_printable_breakpoints++;
>> + }
>> + }
>>
>> if (opts.addressprint)
>> bkpttbl_chain
>> @@ -5169,16 +5178,16 @@ breakpoint_1 (int bnum, int allflag,
>> annotate_field (3);
>> ui_out_table_header (uiout, 3, ui_left, "enabled", "Enb"); /* 4 */
>> if (opts.addressprint)
>> - {
>> - if (nr_printable_breakpoints > 0)
>> - annotate_field (4);
>> - if (print_address_bits <= 32)
>> - ui_out_table_header (uiout, 10, ui_left,
>> - "addr", "Address"); /* 5 */
>> - else
>> - ui_out_table_header (uiout, 18, ui_left,
>> - "addr", "Address"); /* 5 */
>> - }
>> + {
>> + if (nr_printable_breakpoints > 0)
>> + annotate_field (4);
>> + if (print_address_bits <= 32)
>> + ui_out_table_header (uiout, 10, ui_left,
>> + "addr", "Address"); /* 5 */
>> + else
>> + ui_out_table_header (uiout, 18, ui_left,
>> + "addr", "Address"); /* 5 */
>> + }
>
> Please let's keep big formatting changes separate from
> functional changes. We ask that in patch submissions, so we
> should follow the same practice ourselves. It's really a good
> rule BTW. I think of that as one case of what I call it
> The Principle of Least Reversibility (TM), so explained:
> If by chance this patch needs to be reverted, you don't want
> to revert the whitespace fixes. Hence, you'll want to make
> the whitespace fixes a separate patch. The same rule applies
> to preparatory fixes and changes, and code moves (where if
> you move the code, you should NOT change it in any other way).
>
>> add_info ("breakpoints", breakpoints_info, _("\
>> -Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
>> +Status of user-settable breakpoints listed, or all breakpoints if no argument.\n\
>
> "listed" doesn't sound obviously referring to the spec
> you pass as argument to the command. "listed where? the
> command itself is printing a list." was my thought. Is
> there any other way to spell that?
It's getting hard to describe. Do you have a suggestion?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA] info break/watch/trace use get_number_or_range, take two
2011-02-21 23:00 [RFA] info break/watch/trace use get_number_or_range, take two Michael Snyder
2011-02-22 9:27 ` Pedro Alves
@ 2011-02-22 11:49 ` Eli Zaretskii
2011-02-22 18:33 ` Michael Snyder
1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2011-02-22 11:49 UTC (permalink / raw)
To: Michael Snyder; +Cc: gdb-patches
> Date: Mon, 21 Feb 2011 14:23:28 -0800
> From: Michael Snyder <msnyder@vmware.com>
>
> 2011-02-21 Michael Snyder <msnyder@vmware.com>
>
> * gdb.texinfo (Set Breaks): Add @dots{} to arguments of info break.
> (Set Watchpoints): Add @dots{} to argument of info watchpoints.
> (Listing Tracepoints): Add @dots{} to argument of info tracepoints.
This part is okay, but didn't we decide to add an explanation of
ranges, as in "24-42"?
Also, do we want a NEWS entry for this?
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFA] info break/watch/trace use get_number_or_range, take two
2011-02-22 11:49 ` Eli Zaretskii
@ 2011-02-22 18:33 ` Michael Snyder
2011-02-22 18:41 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Michael Snyder @ 2011-02-22 18:33 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
Eli Zaretskii wrote:
>> Date: Mon, 21 Feb 2011 14:23:28 -0800
>> From: Michael Snyder <msnyder@vmware.com>
>>
>> 2011-02-21 Michael Snyder <msnyder@vmware.com>
>>
>> * gdb.texinfo (Set Breaks): Add @dots{} to arguments of info break.
>> (Set Watchpoints): Add @dots{} to argument of info watchpoints.
>> (Listing Tracepoints): Add @dots{} to argument of info tracepoints.
>
> This part is okay, but didn't we decide to add an explanation of
> ranges, as in "24-42"?
I think so, although I see that discussion as unfinished.
The question is, where to put it and how to refer to it?
> Also, do we want a NEWS entry for this?
Sure.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFA] info break/watch/trace use get_number_or_range, take two
2011-02-22 18:33 ` Michael Snyder
@ 2011-02-22 18:41 ` Pedro Alves
0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2011-02-22 18:41 UTC (permalink / raw)
To: gdb-patches; +Cc: Michael Snyder, Eli Zaretskii
On Tuesday 22 February 2011 18:32:42, Michael Snyder wrote:
> >> 2011-02-21 Michael Snyder <msnyder@vmware.com>
> >>
> >> * gdb.texinfo (Set Breaks): Add @dots{} to arguments of info break.
> >> (Set Watchpoints): Add @dots{} to argument of info watchpoints.
> >> (Listing Tracepoints): Add @dots{} to argument of info tracepoints.
> >
> > This part is okay, but didn't we decide to add an explanation of
> > ranges, as in "24-42"?
>
> I think so, although I see that discussion as unfinished.
> The question is, where to put it and how to refer to it?
It's on my queue to document the "disable display" change.
Of course, if someone wants to do that for me, I certainly
don't mind.
"thread apply" has blurb mentioning ranges like that:
It can be a single thread number, one of the numbers
shown in the first field of the @samp{info threads} display; or it
could be a range of thread numbers, as in @code{2-4}.
That looks pretty clear to me. I was thinking on borrowing it.
Or were you thinking of something more detailed?
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-02-22 19:00 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-21 23:00 [RFA] info break/watch/trace use get_number_or_range, take two Michael Snyder
2011-02-22 9:27 ` Pedro Alves
2011-02-22 10:56 ` Eli Zaretskii
2011-02-22 18:32 ` Michael Snyder
2011-02-22 18:48 ` Eli Zaretskii
2011-02-22 19:02 ` Michael Snyder
2011-02-22 18:31 ` Michael Snyder
2011-02-22 11:49 ` Eli Zaretskii
2011-02-22 18:33 ` Michael Snyder
2011-02-22 18:41 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox