This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/3] Use field_string in more places
* 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
>