* [PATCH 0/3] replace many uses of field_fmt @ 2019-07-01 20:01 Tom Tromey 2019-07-01 20:01 ` [PATCH 1/3] Use field_core_addr in more places Tom Tromey ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Tom Tromey @ 2019-07-01 20:01 UTC (permalink / raw) To: gdb-patches This series replaces many of the uses of field_fmt. I believe it fixes a few minor latent bugs along the way. Tested on x86-64 Fedora 29, though note that this touches files not tested in this configuration. Let me know what you think. Tom ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] Use field_core_addr in more places 2019-07-01 20:01 [PATCH 0/3] replace many uses of field_fmt Tom Tromey @ 2019-07-01 20:01 ` Tom Tromey 2019-07-02 9:40 ` Andrew Burgess 2019-07-01 20:01 ` [PATCH 2/3] Use field_string " Tom Tromey 2019-07-01 20:01 ` [PATCH 3/3] Introduce field_unsigned Tom Tromey 2 siblings, 1 reply; 11+ messages in thread From: Tom Tromey @ 2019-07-01 20:01 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This changes a few spots that use field_fmt to use field_core_addr instead. gdb/ChangeLog 2019-07-01 Tom Tromey <tromey@adacore.com> * target.c (flash_erase_command): Use field_core_addr. * symfile.c (generic_load): Use field_core_addr. * sparc64-linux-tdep.c (sparc64_linux_handle_segmentation_fault): Use field_core_addr. * i386-linux-tdep.c (i386_linux_handle_segmentation_fault): Use field_core_addr. --- gdb/ChangeLog | 9 +++++++++ gdb/i386-linux-tdep.c | 6 +++--- gdb/sparc64-linux-tdep.c | 6 +++--- gdb/symfile.c | 2 +- gdb/target.c | 2 +- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c index 74f429b49af..b5c3ef2b71e 100644 --- a/gdb/i386-linux-tdep.c +++ b/gdb/i386-linux-tdep.c @@ -432,13 +432,13 @@ i386_linux_handle_segmentation_fault (struct gdbarch *gdbarch, uiout->field_string ("sigcode-meaning", _("Lower bound violation")); uiout->text (_(" while accessing address ")); - uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, access)); + uiout->field_core_addr ("bound-access", gdbarch, access); uiout->text (_("\nBounds: [lower = ")); - uiout->field_fmt ("lower-bound", "%s", paddress (gdbarch, lower_bound)); + uiout->field_core_addr ("lower-bound", gdbarch, lower_bound); uiout->text (_(", upper = ")); - uiout->field_fmt ("upper-bound", "%s", paddress (gdbarch, upper_bound)); + uiout->field_core_addr ("upper-bound", gdbarch, upper_bound); uiout->text (_("]")); } diff --git a/gdb/sparc64-linux-tdep.c b/gdb/sparc64-linux-tdep.c index 158db97ec61..563d21e4b4c 100644 --- a/gdb/sparc64-linux-tdep.c +++ b/gdb/sparc64-linux-tdep.c @@ -150,19 +150,19 @@ sparc64_linux_handle_segmentation_fault (struct gdbarch *gdbarch, uiout->text ("\n"); uiout->field_string ("sigcode-meaning", _("ADI disabled")); uiout->text (_(" while accessing address ")); - uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr)); + uiout->field_core_addr ("bound-access", gdbarch, addr); break; case SEGV_ADIDERR: /* disrupting mismatch */ uiout->text ("\n"); uiout->field_string ("sigcode-meaning", _("ADI deferred mismatch")); uiout->text (_(" while accessing address ")); - uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr)); + uiout->field_core_addr ("bound-access", gdbarch, addr); break; case SEGV_ADIPERR: /* precise mismatch */ uiout->text ("\n"); uiout->field_string ("sigcode-meaning", _("ADI precise mismatch")); uiout->text (_(" while accessing address ")); - uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr)); + uiout->field_core_addr ("bound-access", gdbarch, addr); break; default: break; diff --git a/gdb/symfile.c b/gdb/symfile.c index 6f9c81c8b94..58049a544b5 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -2081,7 +2081,7 @@ generic_load (const char *args, int from_tty) CORE_ADDR entry = bfd_get_start_address (loadfile_bfd.get ()); entry = gdbarch_addr_bits_remove (target_gdbarch (), entry); uiout->text ("Start address "); - uiout->field_fmt ("address", "%s", paddress (target_gdbarch (), entry)); + uiout->field_core_addr ("address", target_gdbarch (), entry); uiout->text (", load size "); uiout->field_fmt ("load-size", "%lu", total_progress.data_count); uiout->text ("\n"); diff --git a/gdb/target.c b/gdb/target.c index 1c04095fd63..febb3390339 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -3793,7 +3793,7 @@ flash_erase_command (const char *cmd, int from_tty) ui_out_emit_tuple tuple_emitter (current_uiout, "erased-regions"); current_uiout->message (_("Erasing flash memory region at address ")); - current_uiout->field_fmt ("address", "%s", paddress (gdbarch, m.lo)); + current_uiout->field_core_addr ("address", gdbarch, m.lo); current_uiout->message (", size = "); current_uiout->field_fmt ("size", "%s", hex_string (m.hi - m.lo)); current_uiout->message ("\n"); -- 2.20.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Use field_core_addr in more places 2019-07-01 20:01 ` [PATCH 1/3] Use field_core_addr in more places Tom Tromey @ 2019-07-02 9:40 ` Andrew Burgess 0 siblings, 0 replies; 11+ messages in thread From: Andrew Burgess @ 2019-07-02 9:40 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches * Tom Tromey <tromey@adacore.com> [2019-07-01 14:00:56 -0600]: > This changes a few spots that use field_fmt to use field_core_addr > instead. > > gdb/ChangeLog > 2019-07-01 Tom Tromey <tromey@adacore.com> > > * target.c (flash_erase_command): Use field_core_addr. > * symfile.c (generic_load): Use field_core_addr. > * sparc64-linux-tdep.c (sparc64_linux_handle_segmentation_fault): > Use field_core_addr. > * i386-linux-tdep.c (i386_linux_handle_segmentation_fault): Use > field_core_addr. Looks good to me. Thanks, Andrew > --- > gdb/ChangeLog | 9 +++++++++ > gdb/i386-linux-tdep.c | 6 +++--- > gdb/sparc64-linux-tdep.c | 6 +++--- > gdb/symfile.c | 2 +- > gdb/target.c | 2 +- > 5 files changed, 17 insertions(+), 8 deletions(-) > > diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c > index 74f429b49af..b5c3ef2b71e 100644 > --- a/gdb/i386-linux-tdep.c > +++ b/gdb/i386-linux-tdep.c > @@ -432,13 +432,13 @@ i386_linux_handle_segmentation_fault (struct gdbarch *gdbarch, > uiout->field_string ("sigcode-meaning", _("Lower bound violation")); > > uiout->text (_(" while accessing address ")); > - uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, access)); > + uiout->field_core_addr ("bound-access", gdbarch, access); > > uiout->text (_("\nBounds: [lower = ")); > - uiout->field_fmt ("lower-bound", "%s", paddress (gdbarch, lower_bound)); > + uiout->field_core_addr ("lower-bound", gdbarch, lower_bound); > > uiout->text (_(", upper = ")); > - uiout->field_fmt ("upper-bound", "%s", paddress (gdbarch, upper_bound)); > + uiout->field_core_addr ("upper-bound", gdbarch, upper_bound); > > uiout->text (_("]")); > } > diff --git a/gdb/sparc64-linux-tdep.c b/gdb/sparc64-linux-tdep.c > index 158db97ec61..563d21e4b4c 100644 > --- a/gdb/sparc64-linux-tdep.c > +++ b/gdb/sparc64-linux-tdep.c > @@ -150,19 +150,19 @@ sparc64_linux_handle_segmentation_fault (struct gdbarch *gdbarch, > uiout->text ("\n"); > uiout->field_string ("sigcode-meaning", _("ADI disabled")); > uiout->text (_(" while accessing address ")); > - uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr)); > + uiout->field_core_addr ("bound-access", gdbarch, addr); > break; > case SEGV_ADIDERR: /* disrupting mismatch */ > uiout->text ("\n"); > uiout->field_string ("sigcode-meaning", _("ADI deferred mismatch")); > uiout->text (_(" while accessing address ")); > - uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr)); > + uiout->field_core_addr ("bound-access", gdbarch, addr); > break; > case SEGV_ADIPERR: /* precise mismatch */ > uiout->text ("\n"); > uiout->field_string ("sigcode-meaning", _("ADI precise mismatch")); > uiout->text (_(" while accessing address ")); > - uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr)); > + uiout->field_core_addr ("bound-access", gdbarch, addr); > break; > default: > break; > diff --git a/gdb/symfile.c b/gdb/symfile.c > index 6f9c81c8b94..58049a544b5 100644 > --- a/gdb/symfile.c > +++ b/gdb/symfile.c > @@ -2081,7 +2081,7 @@ generic_load (const char *args, int from_tty) > CORE_ADDR entry = bfd_get_start_address (loadfile_bfd.get ()); > entry = gdbarch_addr_bits_remove (target_gdbarch (), entry); > uiout->text ("Start address "); > - uiout->field_fmt ("address", "%s", paddress (target_gdbarch (), entry)); > + uiout->field_core_addr ("address", target_gdbarch (), entry); > uiout->text (", load size "); > uiout->field_fmt ("load-size", "%lu", total_progress.data_count); > uiout->text ("\n"); > diff --git a/gdb/target.c b/gdb/target.c > index 1c04095fd63..febb3390339 100644 > --- a/gdb/target.c > +++ b/gdb/target.c > @@ -3793,7 +3793,7 @@ flash_erase_command (const char *cmd, int from_tty) > ui_out_emit_tuple tuple_emitter (current_uiout, "erased-regions"); > > current_uiout->message (_("Erasing flash memory region at address ")); > - current_uiout->field_fmt ("address", "%s", paddress (gdbarch, m.lo)); > + current_uiout->field_core_addr ("address", gdbarch, m.lo); > current_uiout->message (", size = "); > current_uiout->field_fmt ("size", "%s", hex_string (m.hi - m.lo)); > current_uiout->message ("\n"); > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] Use field_string in more places 2019-07-01 20:01 [PATCH 0/3] replace many uses of field_fmt Tom Tromey 2019-07-01 20:01 ` [PATCH 1/3] Use field_core_addr in more places Tom Tromey @ 2019-07-01 20:01 ` Tom Tromey 2019-07-02 9:39 ` Andrew Burgess 2019-07-01 20:01 ` [PATCH 3/3] Introduce field_unsigned Tom Tromey 2 siblings, 1 reply; 11+ messages in thread From: Tom Tromey @ 2019-07-01 20:01 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This replaces uses of field_fmt with a "%s" format string to use field_string instead. I think this fixes some latent bugs, in cases like: - current_uiout->field_fmt ("event_status", "0x%s", phex (event_status, 4)); Here, I think "0x" will be printed twice. gdb/ChangeLog 2019-07-01 Tom Tromey <tromey@adacore.com> * mi/mi-main.c (list_available_thread_groups): Use field_string. * mi/mi-interp.c (mi_memory_changed): Use field_string. * target.c (flash_erase_command): Use field_string. * spu-tdep.c (info_spu_event_command, info_spu_signal_command) (info_spu_mailbox_list, info_spu_dma_cmdlist) (info_spu_dma_command, info_spu_proxydma_command): Use field_string. * infrun.c (print_signal_received_reason): Use field_string. * i386-tdep.c (i386_mpx_print_bounds): Use field_string. * breakpoint.c (maybe_print_thread_hit_breakpoint): Use field_string. * ada-tasks.c (print_ada_task_info): Use field_string. --- gdb/ChangeLog | 15 +++++++++++++++ gdb/ada-tasks.c | 7 +++---- gdb/breakpoint.c | 4 ++-- gdb/i386-tdep.c | 2 +- gdb/infrun.c | 4 ++-- gdb/mi/mi-interp.c | 2 +- gdb/mi/mi-main.c | 2 +- gdb/spu-tdep.c | 44 ++++++++++++++++++++++---------------------- gdb/target.c | 2 +- 9 files changed, 48 insertions(+), 34 deletions(-) diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c index 9c07f0ca226..a8fcc8ed43b 100644 --- a/gdb/ada-tasks.c +++ b/gdb/ada-tasks.c @@ -1129,10 +1129,9 @@ print_ada_task_info (struct ui_out *uiout, uiout->field_string ("state", task_states[task_info->state]); /* Finally, print the task name. */ - uiout->field_fmt ("name", - "%s", - task_info->name[0] != '\0' ? task_info->name - : _("<no name>")); + uiout->field_string ("name", + task_info->name[0] != '\0' ? task_info->name + : _("<no name>")); uiout->text ("\n"); } diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 8422db8b571..5cdebddfc5d 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -4490,13 +4490,13 @@ maybe_print_thread_hit_breakpoint (struct ui_out *uiout) struct thread_info *thr = inferior_thread (); uiout->text ("Thread "); - uiout->field_fmt ("thread-id", "%s", print_thread_id (thr)); + uiout->field_string ("thread-id", print_thread_id (thr)); name = thr->name != NULL ? thr->name : target_thread_name (thr); if (name != NULL) { uiout->text (" \""); - uiout->field_fmt ("name", "%s", name); + uiout->field_string ("name", name); uiout->text ("\""); } diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index 00c1f8d7499..42fb8b26463 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -8914,7 +8914,7 @@ i386_mpx_print_bounds (const CORE_ADDR bt_entry[4]) size = (size > -1 ? size + 1 : size); uiout->text (", size = "); - uiout->field_fmt ("size", "%s", plongest (size)); + uiout->field_string ("size", plongest (size)); uiout->text (", metadata = "); uiout->field_core_addr ("metadata", gdbarch, bt_entry[3]); diff --git a/gdb/infrun.c b/gdb/infrun.c index 4fd92f1bac2..f1f10fd30ff 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -7690,13 +7690,13 @@ print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal) const char *name; uiout->text ("\nThread "); - uiout->field_fmt ("thread-id", "%s", print_thread_id (thr)); + uiout->field_string ("thread-id", print_thread_id (thr)); name = thr->name != NULL ? thr->name : target_thread_name (thr); if (name != NULL) { uiout->text (" \""); - uiout->field_fmt ("name", "%s", name); + uiout->field_string ("name", name); uiout->text ("\""); } } diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index ad1a06cae0b..76583ff494b 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -1172,7 +1172,7 @@ mi_memory_changed (struct inferior *inferior, CORE_ADDR memaddr, mi_uiout->field_fmt ("thread-group", "i%d", inferior->num); mi_uiout->field_core_addr ("addr", target_gdbarch (), memaddr); - mi_uiout->field_fmt ("len", "%s", hex_string (len)); + mi_uiout->field_string ("len", hex_string (len)); /* Append 'type=code' into notification if MEMADDR falls in the range of sections contain code. */ diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 13c310d494c..dc4feedb6f4 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -746,7 +746,7 @@ list_available_thread_groups (const std::set<int> &ids, int recurse) ui_out_emit_tuple tuple_emitter (uiout, NULL); - uiout->field_fmt ("id", "%s", pid->c_str ()); + uiout->field_string ("id", pid->c_str ()); uiout->field_string ("type", "process"); if (cmd) uiout->field_string ("description", cmd->c_str ()); diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c index a2ac3149d4d..3ac709c1611 100644 --- a/gdb/spu-tdep.c +++ b/gdb/spu-tdep.c @@ -2096,10 +2096,10 @@ info_spu_event_command (const char *args, int from_tty) ui_out_emit_tuple tuple_emitter (current_uiout, "SPUInfoEvent"); current_uiout->text (_("Event Status ")); - current_uiout->field_fmt ("event_status", "0x%s", phex (event_status, 4)); + current_uiout->field_string ("event_status", phex (event_status, 4)); current_uiout->text ("\n"); current_uiout->text (_("Event Mask ")); - current_uiout->field_fmt ("event_mask", "0x%s", phex (event_mask, 4)); + current_uiout->field_string ("event_mask", phex (event_mask, 4)); current_uiout->text ("\n"); } @@ -2168,10 +2168,10 @@ info_spu_signal_command (const char *args, int from_tty) if (current_uiout->is_mi_like_p ()) { current_uiout->field_int ("signal1_pending", signal1_pending); - current_uiout->field_fmt ("signal1", "0x%s", phex_nz (signal1, 4)); + current_uiout->field_string ("signal1", phex (signal1, 4)); current_uiout->field_int ("signal1_type", signal1_type); current_uiout->field_int ("signal2_pending", signal2_pending); - current_uiout->field_fmt ("signal2", "0x%s", phex_nz (signal2, 4)); + current_uiout->field_string ("signal2", phex (signal2, 4)); current_uiout->field_int ("signal2_type", signal2_type); } else @@ -2218,7 +2218,7 @@ info_spu_mailbox_list (gdb_byte *buf, int nr, enum bfd_endian byte_order, ULONGEST val; ui_out_emit_tuple tuple_emitter (current_uiout, "mbox"); val = extract_unsigned_integer (buf + 4*i, 4, byte_order); - current_uiout->field_fmt (field, "0x%s", phex (val, 4)); + current_uiout->field_string (field, phex (val, 4)); } current_uiout->text ("\n"); @@ -2421,7 +2421,7 @@ info_spu_dma_cmdlist (gdb_byte *buf, int nr, enum bfd_endian byte_order) current_uiout->field_int ("rid", rclass_id); if (ea_valid_p) - current_uiout->field_fmt ("ea", "0x%s", phex (mfc_ea, 8)); + current_uiout->field_string ("ea", phex (mfc_ea, 8)); else current_uiout->field_skip ("ea"); @@ -2494,16 +2494,16 @@ info_spu_dma_command (const char *args, int from_tty) if (current_uiout->is_mi_like_p ()) { - current_uiout->field_fmt ("dma_info_type", "0x%s", - phex_nz (dma_info_type, 4)); - current_uiout->field_fmt ("dma_info_mask", "0x%s", - phex_nz (dma_info_mask, 4)); - current_uiout->field_fmt ("dma_info_status", "0x%s", - phex_nz (dma_info_status, 4)); - current_uiout->field_fmt ("dma_info_stall_and_notify", "0x%s", - phex_nz (dma_info_stall_and_notify, 4)); - current_uiout->field_fmt ("dma_info_atomic_command_status", "0x%s", - phex_nz (dma_info_atomic_command_status, 4)); + current_uiout->field_string ("dma_info_type", + phex (dma_info_type, 4)); + current_uiout->field_string ("dma_info_mask", + phex (dma_info_mask, 4)); + current_uiout->field_string ("dma_info_status", + phex (dma_info_status, 4)); + current_uiout->field_string ("dma_info_stall_and_notify", + phex (dma_info_stall_and_notify, 4)); + current_uiout->field_string ("dma_info_atomic_command_status", + phex (dma_info_atomic_command_status, 4)); } else { @@ -2564,12 +2564,12 @@ info_spu_proxydma_command (const char *args, int from_tty) if (current_uiout->is_mi_like_p ()) { - current_uiout->field_fmt ("proxydma_info_type", "0x%s", - phex_nz (dma_info_type, 4)); - current_uiout->field_fmt ("proxydma_info_mask", "0x%s", - phex_nz (dma_info_mask, 4)); - current_uiout->field_fmt ("proxydma_info_status", "0x%s", - phex_nz (dma_info_status, 4)); + current_uiout->field_string ("proxydma_info_type", + phex (dma_info_type, 4)); + current_uiout->field_string ("proxydma_info_mask", + phex (dma_info_mask, 4)); + current_uiout->field_string ("proxydma_info_status", + phex (dma_info_status, 4)); } else { diff --git a/gdb/target.c b/gdb/target.c index febb3390339..417b795d47f 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -3795,7 +3795,7 @@ flash_erase_command (const char *cmd, int from_tty) current_uiout->message (_("Erasing flash memory region at address ")); current_uiout->field_core_addr ("address", gdbarch, m.lo); current_uiout->message (", size = "); - current_uiout->field_fmt ("size", "%s", hex_string (m.hi - m.lo)); + current_uiout->field_string ("size", hex_string (m.hi - m.lo)); current_uiout->message ("\n"); } } -- 2.20.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Use field_string in more places 2019-07-01 20:01 ` [PATCH 2/3] Use field_string " Tom Tromey @ 2019-07-02 9:39 ` Andrew Burgess 2019-07-02 12:24 ` Tom Tromey 0 siblings, 1 reply; 11+ messages in thread From: Andrew Burgess @ 2019-07-02 9:39 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches * Tom Tromey <tromey@adacore.com> [2019-07-01 14:00:57 -0600]: > This replaces uses of field_fmt with a "%s" format string to use > field_string instead. > > I think this fixes some latent bugs, in cases like: > > - current_uiout->field_fmt ("event_status", "0x%s", phex (event_status, 4)); > > Here, I think "0x" will be printed twice. I don't think this is correct, at least experimenting on HEAD for me doesn't print it twice. I think you could replace phex with hex_string_custom to add the '0x' for you, except that hex_string_custom seems to treat the width differently to phex. phex treats the width as a byte width, while hex_string_custom treats the width as a character width, so phex (event_status, 4) needs to become hex_string_custom (event_status, 8) Thanks, Andrew > > gdb/ChangeLog > 2019-07-01 Tom Tromey <tromey@adacore.com> > > * mi/mi-main.c (list_available_thread_groups): Use field_string. > * mi/mi-interp.c (mi_memory_changed): Use field_string. > * target.c (flash_erase_command): Use field_string. > * spu-tdep.c (info_spu_event_command, info_spu_signal_command) > (info_spu_mailbox_list, info_spu_dma_cmdlist) > (info_spu_dma_command, info_spu_proxydma_command): Use > field_string. > * infrun.c (print_signal_received_reason): Use field_string. > * i386-tdep.c (i386_mpx_print_bounds): Use field_string. > * breakpoint.c (maybe_print_thread_hit_breakpoint): Use > field_string. > * ada-tasks.c (print_ada_task_info): Use field_string. > --- > gdb/ChangeLog | 15 +++++++++++++++ > gdb/ada-tasks.c | 7 +++---- > gdb/breakpoint.c | 4 ++-- > gdb/i386-tdep.c | 2 +- > gdb/infrun.c | 4 ++-- > gdb/mi/mi-interp.c | 2 +- > gdb/mi/mi-main.c | 2 +- > gdb/spu-tdep.c | 44 ++++++++++++++++++++++---------------------- > gdb/target.c | 2 +- > 9 files changed, 48 insertions(+), 34 deletions(-) > > diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c > index 9c07f0ca226..a8fcc8ed43b 100644 > --- a/gdb/ada-tasks.c > +++ b/gdb/ada-tasks.c > @@ -1129,10 +1129,9 @@ print_ada_task_info (struct ui_out *uiout, > uiout->field_string ("state", task_states[task_info->state]); > > /* Finally, print the task name. */ > - uiout->field_fmt ("name", > - "%s", > - task_info->name[0] != '\0' ? task_info->name > - : _("<no name>")); > + uiout->field_string ("name", > + task_info->name[0] != '\0' ? task_info->name > + : _("<no name>")); > > uiout->text ("\n"); > } > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index 8422db8b571..5cdebddfc5d 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -4490,13 +4490,13 @@ maybe_print_thread_hit_breakpoint (struct ui_out *uiout) > struct thread_info *thr = inferior_thread (); > > uiout->text ("Thread "); > - uiout->field_fmt ("thread-id", "%s", print_thread_id (thr)); > + uiout->field_string ("thread-id", print_thread_id (thr)); > > name = thr->name != NULL ? thr->name : target_thread_name (thr); > if (name != NULL) > { > uiout->text (" \""); > - uiout->field_fmt ("name", "%s", name); > + uiout->field_string ("name", name); > uiout->text ("\""); > } > > diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c > index 00c1f8d7499..42fb8b26463 100644 > --- a/gdb/i386-tdep.c > +++ b/gdb/i386-tdep.c > @@ -8914,7 +8914,7 @@ i386_mpx_print_bounds (const CORE_ADDR bt_entry[4]) > > size = (size > -1 ? size + 1 : size); > uiout->text (", size = "); > - uiout->field_fmt ("size", "%s", plongest (size)); > + uiout->field_string ("size", plongest (size)); > > uiout->text (", metadata = "); > uiout->field_core_addr ("metadata", gdbarch, bt_entry[3]); > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 4fd92f1bac2..f1f10fd30ff 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -7690,13 +7690,13 @@ print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal) > const char *name; > > uiout->text ("\nThread "); > - uiout->field_fmt ("thread-id", "%s", print_thread_id (thr)); > + uiout->field_string ("thread-id", print_thread_id (thr)); > > name = thr->name != NULL ? thr->name : target_thread_name (thr); > if (name != NULL) > { > uiout->text (" \""); > - uiout->field_fmt ("name", "%s", name); > + uiout->field_string ("name", name); > uiout->text ("\""); > } > } > diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c > index ad1a06cae0b..76583ff494b 100644 > --- a/gdb/mi/mi-interp.c > +++ b/gdb/mi/mi-interp.c > @@ -1172,7 +1172,7 @@ mi_memory_changed (struct inferior *inferior, CORE_ADDR memaddr, > > mi_uiout->field_fmt ("thread-group", "i%d", inferior->num); > mi_uiout->field_core_addr ("addr", target_gdbarch (), memaddr); > - mi_uiout->field_fmt ("len", "%s", hex_string (len)); > + mi_uiout->field_string ("len", hex_string (len)); > > /* Append 'type=code' into notification if MEMADDR falls in the range of > sections contain code. */ > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index 13c310d494c..dc4feedb6f4 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -746,7 +746,7 @@ list_available_thread_groups (const std::set<int> &ids, int recurse) > > ui_out_emit_tuple tuple_emitter (uiout, NULL); > > - uiout->field_fmt ("id", "%s", pid->c_str ()); > + uiout->field_string ("id", pid->c_str ()); > uiout->field_string ("type", "process"); > if (cmd) > uiout->field_string ("description", cmd->c_str ()); > diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c > index a2ac3149d4d..3ac709c1611 100644 > --- a/gdb/spu-tdep.c > +++ b/gdb/spu-tdep.c > @@ -2096,10 +2096,10 @@ info_spu_event_command (const char *args, int from_tty) > ui_out_emit_tuple tuple_emitter (current_uiout, "SPUInfoEvent"); > > current_uiout->text (_("Event Status ")); > - current_uiout->field_fmt ("event_status", "0x%s", phex (event_status, 4)); > + current_uiout->field_string ("event_status", phex (event_status, 4)); > current_uiout->text ("\n"); > current_uiout->text (_("Event Mask ")); > - current_uiout->field_fmt ("event_mask", "0x%s", phex (event_mask, 4)); > + current_uiout->field_string ("event_mask", phex (event_mask, 4)); > current_uiout->text ("\n"); > } > > @@ -2168,10 +2168,10 @@ info_spu_signal_command (const char *args, int from_tty) > if (current_uiout->is_mi_like_p ()) > { > current_uiout->field_int ("signal1_pending", signal1_pending); > - current_uiout->field_fmt ("signal1", "0x%s", phex_nz (signal1, 4)); > + current_uiout->field_string ("signal1", phex (signal1, 4)); > current_uiout->field_int ("signal1_type", signal1_type); > current_uiout->field_int ("signal2_pending", signal2_pending); > - current_uiout->field_fmt ("signal2", "0x%s", phex_nz (signal2, 4)); > + current_uiout->field_string ("signal2", phex (signal2, 4)); > current_uiout->field_int ("signal2_type", signal2_type); > } > else > @@ -2218,7 +2218,7 @@ info_spu_mailbox_list (gdb_byte *buf, int nr, enum bfd_endian byte_order, > ULONGEST val; > ui_out_emit_tuple tuple_emitter (current_uiout, "mbox"); > val = extract_unsigned_integer (buf + 4*i, 4, byte_order); > - current_uiout->field_fmt (field, "0x%s", phex (val, 4)); > + current_uiout->field_string (field, phex (val, 4)); > } > > current_uiout->text ("\n"); > @@ -2421,7 +2421,7 @@ info_spu_dma_cmdlist (gdb_byte *buf, int nr, enum bfd_endian byte_order) > current_uiout->field_int ("rid", rclass_id); > > if (ea_valid_p) > - current_uiout->field_fmt ("ea", "0x%s", phex (mfc_ea, 8)); > + current_uiout->field_string ("ea", phex (mfc_ea, 8)); > else > current_uiout->field_skip ("ea"); > > @@ -2494,16 +2494,16 @@ info_spu_dma_command (const char *args, int from_tty) > > if (current_uiout->is_mi_like_p ()) > { > - current_uiout->field_fmt ("dma_info_type", "0x%s", > - phex_nz (dma_info_type, 4)); > - current_uiout->field_fmt ("dma_info_mask", "0x%s", > - phex_nz (dma_info_mask, 4)); > - current_uiout->field_fmt ("dma_info_status", "0x%s", > - phex_nz (dma_info_status, 4)); > - current_uiout->field_fmt ("dma_info_stall_and_notify", "0x%s", > - phex_nz (dma_info_stall_and_notify, 4)); > - current_uiout->field_fmt ("dma_info_atomic_command_status", "0x%s", > - phex_nz (dma_info_atomic_command_status, 4)); > + current_uiout->field_string ("dma_info_type", > + phex (dma_info_type, 4)); > + current_uiout->field_string ("dma_info_mask", > + phex (dma_info_mask, 4)); > + current_uiout->field_string ("dma_info_status", > + phex (dma_info_status, 4)); > + current_uiout->field_string ("dma_info_stall_and_notify", > + phex (dma_info_stall_and_notify, 4)); > + current_uiout->field_string ("dma_info_atomic_command_status", > + phex (dma_info_atomic_command_status, 4)); > } > else > { > @@ -2564,12 +2564,12 @@ info_spu_proxydma_command (const char *args, int from_tty) > > if (current_uiout->is_mi_like_p ()) > { > - current_uiout->field_fmt ("proxydma_info_type", "0x%s", > - phex_nz (dma_info_type, 4)); > - current_uiout->field_fmt ("proxydma_info_mask", "0x%s", > - phex_nz (dma_info_mask, 4)); > - current_uiout->field_fmt ("proxydma_info_status", "0x%s", > - phex_nz (dma_info_status, 4)); > + current_uiout->field_string ("proxydma_info_type", > + phex (dma_info_type, 4)); > + current_uiout->field_string ("proxydma_info_mask", > + phex (dma_info_mask, 4)); > + current_uiout->field_string ("proxydma_info_status", > + phex (dma_info_status, 4)); > } > else > { > diff --git a/gdb/target.c b/gdb/target.c > index febb3390339..417b795d47f 100644 > --- a/gdb/target.c > +++ b/gdb/target.c > @@ -3795,7 +3795,7 @@ flash_erase_command (const char *cmd, int from_tty) > current_uiout->message (_("Erasing flash memory region at address ")); > current_uiout->field_core_addr ("address", gdbarch, m.lo); > current_uiout->message (", size = "); > - current_uiout->field_fmt ("size", "%s", hex_string (m.hi - m.lo)); > + current_uiout->field_string ("size", hex_string (m.hi - m.lo)); > current_uiout->message ("\n"); > } > } > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Use field_string in more places 2019-07-02 9:39 ` Andrew Burgess @ 2019-07-02 12:24 ` Tom Tromey 0 siblings, 0 replies; 11+ messages in thread From: Tom Tromey @ 2019-07-02 12:24 UTC (permalink / raw) To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: Andrew> I don't think this is correct, at least experimenting on HEAD for me Andrew> doesn't print it twice. Aha, thanks. I will update everything. I think I always get confused by the naming here and think that "phex_nz" implies "no 0x", but it really doesn't. Tom ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] Introduce field_unsigned 2019-07-01 20:01 [PATCH 0/3] replace many uses of field_fmt Tom Tromey 2019-07-01 20:01 ` [PATCH 1/3] Use field_core_addr in more places Tom Tromey 2019-07-01 20:01 ` [PATCH 2/3] Use field_string " Tom Tromey @ 2019-07-01 20:01 ` Tom Tromey 2019-07-02 6:43 ` Metzger, Markus T 2 siblings, 1 reply; 11+ messages in thread From: Tom Tromey @ 2019-07-01 20:01 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This adds field_unsigned and changes various places using field_fmt with "%u" to use this instead. This also replaces an existing equivalent helper function in record-btrace.c. gdb/ChangeLog 2019-07-01 Tom Tromey <tromey@adacore.com> * symfile.c (generic_load): Use field_unsigned. (print_transfer_performance): Likewise. * record-btrace.c (ui_out_field_uint): Remove. (btrace_call_history_insn_range, btrace_call_history): Use field_unsigned. * disasm.c (gdb_pretty_print_disassembler::pretty_print_insn): Use field_unsigned. * ui-out.h (class ui_out) <field_unsigned>: New method. --- gdb/ChangeLog | 11 +++++++++++ gdb/disasm.c | 2 +- gdb/record-btrace.c | 14 +++----------- gdb/symfile.c | 12 ++++++------ gdb/ui-out.h | 6 ++++++ 5 files changed, 27 insertions(+), 18 deletions(-) diff --git a/gdb/disasm.c b/gdb/disasm.c index 4e58cb67f93..927b092ce2c 100644 --- a/gdb/disasm.c +++ b/gdb/disasm.c @@ -209,7 +209,7 @@ gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout, if (insn->number != 0) { - uiout->field_fmt ("insn-number", "%u", insn->number); + uiout->field_unsigned ("insn-number", insn->number); uiout->text ("\t"); } diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c index 21085d5c62c..313f1ac7d1e 100644 --- a/gdb/record-btrace.c +++ b/gdb/record-btrace.c @@ -623,14 +623,6 @@ btrace_ui_out_decode_error (struct ui_out *uiout, int errcode, uiout->text (_("]\n")); } -/* Print an unsigned int. */ - -static void -ui_out_field_uint (struct ui_out *uiout, const char *fld, unsigned int val) -{ - uiout->field_fmt (fld, "%u", val); -} - /* A range of source lines. */ struct btrace_line_range @@ -1032,9 +1024,9 @@ btrace_call_history_insn_range (struct ui_out *uiout, begin = bfun->insn_offset; end = begin + size - 1; - ui_out_field_uint (uiout, "insn begin", begin); + uiout->field_unsigned ("insn begin", begin); uiout->text (","); - ui_out_field_uint (uiout, "insn end", end); + uiout->field_unsigned ("insn end", end); } /* Compute the lowest and highest source line for the instructions in BFUN @@ -1155,7 +1147,7 @@ btrace_call_history (struct ui_out *uiout, msym = bfun->msym; /* Print the function index. */ - ui_out_field_uint (uiout, "index", bfun->number); + uiout->field_unsigned ("index", bfun->number); uiout->text ("\t"); /* Indicate gaps in the trace. */ diff --git a/gdb/symfile.c b/gdb/symfile.c index 58049a544b5..9b71ad6b3a4 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -2083,7 +2083,7 @@ generic_load (const char *args, int from_tty) uiout->text ("Start address "); uiout->field_core_addr ("address", target_gdbarch (), entry); uiout->text (", load size "); - uiout->field_fmt ("load-size", "%lu", total_progress.data_count); + uiout->field_unsigned ("load-size", total_progress.data_count); uiout->text ("\n"); regcache_write_pc (get_current_regcache (), entry); @@ -2126,29 +2126,29 @@ print_transfer_performance (struct ui_file *stream, if (uiout->is_mi_like_p ()) { - uiout->field_fmt ("transfer-rate", "%lu", rate * 8); + uiout->field_unsigned ("transfer-rate", rate * 8); uiout->text (" bits/sec"); } else if (rate < 1024) { - uiout->field_fmt ("transfer-rate", "%lu", rate); + uiout->field_unsigned ("transfer-rate", rate); uiout->text (" bytes/sec"); } else { - uiout->field_fmt ("transfer-rate", "%lu", rate / 1024); + uiout->field_unsigned ("transfer-rate", rate / 1024); uiout->text (" KB/sec"); } } else { - uiout->field_fmt ("transferred-bits", "%lu", (data_count * 8)); + uiout->field_unsigned ("transferred-bits", (data_count * 8)); uiout->text (" bits in <1 sec"); } if (write_count > 0) { uiout->text (", "); - uiout->field_fmt ("write-rate", "%lu", data_count / write_count); + uiout->field_unsigned ("write-rate", data_count / write_count); uiout->text (" bytes/write"); } uiout->text (".\n"); diff --git a/gdb/ui-out.h b/gdb/ui-out.h index 9eba70eedac..cf074ebcef4 100644 --- a/gdb/ui-out.h +++ b/gdb/ui-out.h @@ -121,6 +121,12 @@ class ui_out void field_fmt (const char *fldname, const char *format, ...) ATTRIBUTE_PRINTF (3, 4); + /* Like field_int, but print an unsigned value. */ + void field_unsigned (const char *fldname, ULONGEST value) + { + field_string (fldname, pulongest (value)); + } + void spaces (int numspaces); void text (const char *string); void message (const char *format, ...) ATTRIBUTE_PRINTF (2, 3); -- 2.20.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 3/3] Introduce field_unsigned 2019-07-01 20:01 ` [PATCH 3/3] Introduce field_unsigned Tom Tromey @ 2019-07-02 6:43 ` Metzger, Markus T 2019-07-02 12:23 ` Tom Tromey 0 siblings, 1 reply; 11+ messages in thread From: Metzger, Markus T @ 2019-07-02 6:43 UTC (permalink / raw) To: Tom Tromey, gdb-patches Hello Tom, > This adds field_unsigned and changes various places using field_fmt > with "%u" to use this instead. This also replaces an existing > equivalent helper function in record-btrace.c. > > gdb/ChangeLog > 2019-07-01 Tom Tromey <tromey@adacore.com> > > * symfile.c (generic_load): Use field_unsigned. > (print_transfer_performance): Likewise. > * record-btrace.c (ui_out_field_uint): Remove. > (btrace_call_history_insn_range, btrace_call_history): Use > field_unsigned. > * disasm.c (gdb_pretty_print_disassembler::pretty_print_insn): Use > field_unsigned. > * ui-out.h (class ui_out) <field_unsigned>: New method. > --- > gdb/ChangeLog | 11 +++++++++++ > gdb/disasm.c | 2 +- > gdb/record-btrace.c | 14 +++----------- > gdb/symfile.c | 12 ++++++------ > gdb/ui-out.h | 6 ++++++ > 5 files changed, 27 insertions(+), 18 deletions(-) > diff --git a/gdb/ui-out.h b/gdb/ui-out.h > index 9eba70eedac..cf074ebcef4 100644 > --- a/gdb/ui-out.h > +++ b/gdb/ui-out.h > @@ -121,6 +121,12 @@ class ui_out > void field_fmt (const char *fldname, const char *format, ...) > ATTRIBUTE_PRINTF (3, 4); > > + /* Like field_int, but print an unsigned value. */ > + void field_unsigned (const char *fldname, ULONGEST value) > + { > + field_string (fldname, pulongest (value)); > + } > + > void spaces (int numspaces); > void text (const char *string); > void message (const char *format, ...) ATTRIBUTE_PRINTF (2, 3); This is quite different from how other types are handled, e.g. ui_out::field_int (const char *fldname, int value) { int fldno; int width; ui_align align; verify_field (&fldno, &width, &align); do_field_int (fldno, width, align, fldname, value); } Is there a reason for not treating int and unsigned int in a similar way? The rest looks good to me. Regards, Markus. Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Gary Kershaw Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] Introduce field_unsigned 2019-07-02 6:43 ` Metzger, Markus T @ 2019-07-02 12:23 ` Tom Tromey 2019-07-02 12:41 ` Metzger, Markus T 0 siblings, 1 reply; 11+ messages in thread From: Tom Tromey @ 2019-07-02 12:23 UTC (permalink / raw) To: Metzger, Markus T; +Cc: Tom Tromey, gdb-patches >> + void field_unsigned (const char *fldname, ULONGEST value) >> + { >> + field_string (fldname, pulongest (value)); >> + } [...] >> This is quite different from how other types are handled, e.g. >> ui_out::field_int (const char *fldname, int value) >> { >> int fldno; >> int width; >> ui_align align; >> verify_field (&fldno, &width, &align); >> do_field_int (fldno, width, align, fldname, value); >> } >> Is there a reason for not treating int and unsigned int in a similar way? There just didn't seem to be a benefit, and the field_int approach is wordier. In the end all the existing do_field_int methods actually just end up in their respective do_field_string methods anyway. E.g., mi-out.c: void mi_ui_out::do_field_int (int fldno, int width, ui_align alignment, const char *fldname, int value) { char buffer[20]; /* FIXME: how many chars long a %d can become? */ xsnprintf (buffer, sizeof (buffer), "%d", value); do_field_string (fldno, width, alignment, fldname, buffer, ui_out_style_kind::DEFAULT); } Here the new approach is actually a bit better because pulongest doesn't have this FIXME. Perhaps we should instead switch field_int to take a LONGEST and follow the approach used in this new patch. Tom ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 3/3] Introduce field_unsigned 2019-07-02 12:23 ` Tom Tromey @ 2019-07-02 12:41 ` Metzger, Markus T 2019-07-02 15:01 ` Tom Tromey 0 siblings, 1 reply; 11+ messages in thread From: Metzger, Markus T @ 2019-07-02 12:41 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches Hello Tom, > In the end all the existing do_field_int methods actually just end up in their > respective do_field_string methods anyway. E.g., mi-out.c: > > void > mi_ui_out::do_field_int (int fldno, int width, ui_align alignment, > const char *fldname, int value) > { > char buffer[20]; /* FIXME: how many chars long a %d can become? */ > > xsnprintf (buffer, sizeof (buffer), "%d", value); > do_field_string (fldno, width, alignment, fldname, buffer, > ui_out_style_kind::DEFAULT); > } > > Here the new approach is actually a bit better because pulongest doesn't have this > FIXME. Perhaps we should instead switch field_int to take a LONGEST and follow > the approach used in this new patch. If subclasses don't need to know that the field was originally an integer or a float, all of ui_out can be reduced to outputting strings and sub-classes need only provide that one member function for all field types. I don't have any preference one way or another, I was mainly commenting on the apparent discrepancy how similar field types are handled. Markus. Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Gary Kershaw Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] Introduce field_unsigned 2019-07-02 12:41 ` Metzger, Markus T @ 2019-07-02 15:01 ` Tom Tromey 0 siblings, 0 replies; 11+ messages in thread From: Tom Tromey @ 2019-07-02 15:01 UTC (permalink / raw) To: Metzger, Markus T; +Cc: Tom Tromey, gdb-patches >> If subclasses don't need to know that the field was originally an integer or a float, >> all of ui_out can be reduced to outputting strings and sub-classes need only provide >> that one member function for all field types. >> I don't have any preference one way or another, I was mainly commenting on the >> apparent discrepancy how similar field types are handled. I suppose if we ever want to, say, switch MI to use JSON, then preserving the difference would make sense. I'll change it. Tom ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-07-02 15:01 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-01 20:01 [PATCH 0/3] replace many uses of field_fmt Tom Tromey 2019-07-01 20:01 ` [PATCH 1/3] Use field_core_addr in more places Tom Tromey 2019-07-02 9:40 ` Andrew Burgess 2019-07-01 20:01 ` [PATCH 2/3] Use field_string " Tom Tromey 2019-07-02 9:39 ` Andrew Burgess 2019-07-02 12:24 ` Tom Tromey 2019-07-01 20:01 ` [PATCH 3/3] Introduce field_unsigned Tom Tromey 2019-07-02 6:43 ` Metzger, Markus T 2019-07-02 12:23 ` Tom Tromey 2019-07-02 12:41 ` Metzger, Markus T 2019-07-02 15:01 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).