[3/3] Introduce field_unsigned
Commit Message
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(-)
Comments
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
>> + 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
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
>> 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
@@ -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");
}
@@ -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. */
@@ -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");
@@ -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);