This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] info break/watch/trace use get_number_or_range, take two


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?



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]