[3/3] Introduce field_unsigned

  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(-)
  

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