[v2] Align natural-format register values to the same column
Commit Message
Currently, commands such as "info reg", "info all-reg", as well as register
window in the TUI print badly aligned columns, like here:
eax 0x1 1
ecx 0xffffd3e0 -11296
edx 0xffffd404 -11260
ebx 0xf7fa5ff4 -134586380
esp 0xffffd390 0xffffd390
ebp 0xffffd3c8 0xffffd3c8
esi 0x0 0
edi 0x0 0
eip 0x8048b60 0x8048b60 <main+16>
eflags 0x286 [ PF SF IF ]
cs 0x23 35
ss 0x2b 43
ds 0x2b 43
es 0x2b 43
fs 0x0 0
gs 0x63 99
After this patch, these commands print the third column values consistently
aligned one under another, provided the second column is not too long.
Originally, the third column was (attempted to be) aligned using a simple tab
character. Lots of tests in the test suite check for it, so this patch retains
the tab in the output after the second column. This allows these tests to
continue working unchanged. What is different is that now the tab may be
followed by several spaces, which complete the task of aligning the third
column when the sole tab doesn't work well.
gdb/ChangeLog:
* infcmd.c (default_print_one_register_info): Align natural-
format column value consistently one after another.
---
gdb/infcmd.c | 35 ++++++++++++++++++++++++++---------
1 file changed, 26 insertions(+), 9 deletions(-)
Comments
Hi Ruslan,
Thanks for the update, I just have some minor comments, but otherwise
the patch looks good to me.
On 2017-07-26 09:53, Ruslan Kabatsayev wrote:
> Currently, commands such as "info reg", "info all-reg", as well as
> register
> window in the TUI print badly aligned columns, like here:
>
> eax 0x1 1
> ecx 0xffffd3e0 -11296
> edx 0xffffd404 -11260
> ebx 0xf7fa5ff4 -134586380
> esp 0xffffd390 0xffffd390
> ebp 0xffffd3c8 0xffffd3c8
> esi 0x0 0
> edi 0x0 0
> eip 0x8048b60 0x8048b60 <main+16>
> eflags 0x286 [ PF SF IF ]
> cs 0x23 35
> ss 0x2b 43
> ds 0x2b 43
> es 0x2b 43
> fs 0x0 0
> gs 0x63 99
>
> After this patch, these commands print the third column values
> consistently
> aligned one under another, provided the second column is not too long.
> Originally, the third column was (attempted to be) aligned using a
> simple tab
> character. Lots of tests in the test suite check for it, so this patch
> retains
> the tab in the output after the second column. This allows these tests
> to
> continue working unchanged. What is different is that now the tab may
> be
> followed by several spaces, which complete the task of aligning the
> third
> column when the sole tab doesn't work well.
>
> gdb/ChangeLog:
>
> * infcmd.c (default_print_one_register_info): Align natural-
> format column value consistently one after another.
> ---
> gdb/infcmd.c | 35 ++++++++++++++++++++++++++---------
> 1 file changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index defa7b0..56cdee2 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -2270,6 +2270,15 @@ path_command (char *dirname, int from_tty)
> }
>
>
Please add a short comment to explain what this function does.
> +static void
> +pad_to_column_prepending_tab (string_file& stream, int col)
The & should go after the space.
> +{
> + const int size_with_tab = stream.size () / 8 * 8 + 8;
> + stream.putc ('\t');
> + if (size_with_tab < col)
> + stream.puts (n_spaces (col - size_with_tab));
> +}
> +
> /* Print out the register NAME with value VAL, to FILE, in the default
> fashion. */
>
> @@ -2280,9 +2289,15 @@ default_print_one_register_info (struct ui_file
> *file,
> {
> struct type *regtype = value_type (val);
> int print_raw_format;
> + string_file format_stream;
> + enum tab_stops
> + {
> + value_column_1=15,
> + value_column_2=32,
Perhaps those could have more explicit names, like "column_hex" and
"column_natural"?
Formatting nit: indent those two lines with two columns less (so with 6
spaces in total) and add spaces around the =.
> + };
>
> - fputs_filtered (name, file);
> - print_spaces_filtered (15 - strlen (name), file);
> + format_stream.puts (name);
> + format_stream.puts (n_spaces (value_column_1 - strlen (name)));
>
> print_raw_format = (value_entirely_available (val)
> && !value_optimized_out (val));
> @@ -2301,14 +2316,15 @@ default_print_one_register_info (struct ui_file
> *file,
>
> val_print (regtype,
> value_embedded_offset (val), 0,
> - file, 0, val, &opts, current_language);
> + &format_stream, 0, val, &opts, current_language);
>
> if (print_raw_format)
> {
> - fprintf_filtered (file, "\t(raw ");
> - print_hex_chars (file, valaddr, TYPE_LENGTH (regtype), byte_order,
> + pad_to_column_prepending_tab (format_stream, value_column_2);
> + format_stream.puts ("(raw ");
> + print_hex_chars (&format_stream, valaddr, TYPE_LENGTH (regtype),
> byte_order,
> true);
> - fprintf_filtered (file, ")");
> + format_stream.puts (")");
> }
> }
> else
> @@ -2320,20 +2336,21 @@ default_print_one_register_info (struct ui_file
> *file,
> opts.deref_ref = 1;
> val_print (regtype,
> value_embedded_offset (val), 0,
> - file, 0, val, &opts, current_language);
> + &format_stream, 0, val, &opts, current_language);
> /* If not a vector register, print it also according to its
> natural format. */
> if (print_raw_format && TYPE_VECTOR (regtype) == 0)
> {
> + pad_to_column_prepending_tab (format_stream, value_column_2);
> get_user_print_options (&opts);
> opts.deref_ref = 1;
> - fprintf_filtered (file, "\t");
> val_print (regtype,
> value_embedded_offset (val), 0,
> - file, 0, val, &opts, current_language);
> + &format_stream, 0, val, &opts, current_language);
> }
> }
>
> + fputs_filtered (format_stream.c_str (), file);
> fprintf_filtered (file, "\n");
> }
Thanks,
Simon
Hi Simon,
On 26.07.2017 12:33, Simon Marchi wrote:
> Hi Ruslan,
>
> Thanks for the update, I just have some minor comments, but otherwise
> the patch looks good to me.
>
> On 2017-07-26 09:53, Ruslan Kabatsayev wrote:
>> Currently, commands such as "info reg", "info all-reg", as well as
>> register
>> window in the TUI print badly aligned columns, like here:
>>
>> eax 0x1 1
>> ecx 0xffffd3e0 -11296
>> edx 0xffffd404 -11260
>> ebx 0xf7fa5ff4 -134586380
>> esp 0xffffd390 0xffffd390
>> ebp 0xffffd3c8 0xffffd3c8
>> esi 0x0 0
>> edi 0x0 0
>> eip 0x8048b60 0x8048b60 <main+16>
>> eflags 0x286 [ PF SF IF ]
>> cs 0x23 35
>> ss 0x2b 43
>> ds 0x2b 43
>> es 0x2b 43
>> fs 0x0 0
>> gs 0x63 99
>>
>> After this patch, these commands print the third column values
>> consistently
>> aligned one under another, provided the second column is not too long.
>> Originally, the third column was (attempted to be) aligned using a
>> simple tab
>> character. Lots of tests in the test suite check for it, so this patch
>> retains
>> the tab in the output after the second column. This allows these tests
>> to
>> continue working unchanged. What is different is that now the tab may
>> be
>> followed by several spaces, which complete the task of aligning the
>> third
>> column when the sole tab doesn't work well.
>>
>> gdb/ChangeLog:
>>
>> * infcmd.c (default_print_one_register_info): Align natural-
>> format column value consistently one after another.
>> ---
>> gdb/infcmd.c | 35 ++++++++++++++++++++++++++---------
>> 1 file changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
>> index defa7b0..56cdee2 100644
>> --- a/gdb/infcmd.c
>> +++ b/gdb/infcmd.c
>> @@ -2270,6 +2270,15 @@ path_command (char *dirname, int from_tty)
>> }
>>
>>
>
> Please add a short comment to explain what this function does.
>
>> +static void
>> +pad_to_column_prepending_tab (string_file& stream, int col)
>
> The & should go after the space.
>
>> +{
>> + const int size_with_tab = stream.size () / 8 * 8 + 8;
>> + stream.putc ('\t');
>> + if (size_with_tab < col)
>> + stream.puts (n_spaces (col - size_with_tab));
>> +}
>> +
>> /* Print out the register NAME with value VAL, to FILE, in the default
>> fashion. */
>>
>> @@ -2280,9 +2289,15 @@ default_print_one_register_info (struct ui_file
>> *file,
>> {
>> struct type *regtype = value_type (val);
>> int print_raw_format;
>> + string_file format_stream;
>> + enum tab_stops
>> + {
>> + value_column_1=15,
>> + value_column_2=32,
>
> Perhaps those could have more explicit names, like "column_hex" and
> "column_natural"?
This was my first thought, but then I noticed that for floating-point
registers we have "(raw 0x....)" on the right column and "1.32536e+52"
on the left one. Do you still think it's OK to name the left column hex
and the right one natural?
>
> Formatting nit: indent those two lines with two columns less (so with 6
> spaces in total) and add spaces around the =.
>
>> + };
>>
>> - fputs_filtered (name, file);
>> - print_spaces_filtered (15 - strlen (name), file);
>> + format_stream.puts (name);
>> + format_stream.puts (n_spaces (value_column_1 - strlen (name)));
>>
>> print_raw_format = (value_entirely_available (val)
>> && !value_optimized_out (val));
>> @@ -2301,14 +2316,15 @@ default_print_one_register_info (struct ui_file
>> *file,
>>
>> val_print (regtype,
>> value_embedded_offset (val), 0,
>> - file, 0, val, &opts, current_language);
>> + &format_stream, 0, val, &opts, current_language);
>>
>> if (print_raw_format)
>> {
>> - fprintf_filtered (file, "\t(raw ");
>> - print_hex_chars (file, valaddr, TYPE_LENGTH (regtype), byte_order,
>> + pad_to_column_prepending_tab (format_stream, value_column_2);
>> + format_stream.puts ("(raw ");
>> + print_hex_chars (&format_stream, valaddr, TYPE_LENGTH (regtype),
>> byte_order,
>> true);
>> - fprintf_filtered (file, ")");
>> + format_stream.puts (")");
>> }
>> }
>> else
>> @@ -2320,20 +2336,21 @@ default_print_one_register_info (struct ui_file
>> *file,
>> opts.deref_ref = 1;
>> val_print (regtype,
>> value_embedded_offset (val), 0,
>> - file, 0, val, &opts, current_language);
>> + &format_stream, 0, val, &opts, current_language);
>> /* If not a vector register, print it also according to its
>> natural format. */
>> if (print_raw_format && TYPE_VECTOR (regtype) == 0)
>> {
>> + pad_to_column_prepending_tab (format_stream, value_column_2);
>> get_user_print_options (&opts);
>> opts.deref_ref = 1;
>> - fprintf_filtered (file, "\t");
>> val_print (regtype,
>> value_embedded_offset (val), 0,
>> - file, 0, val, &opts, current_language);
>> + &format_stream, 0, val, &opts, current_language);
>> }
>> }
>>
>> + fputs_filtered (format_stream.c_str (), file);
>> fprintf_filtered (file, "\n");
>> }
>
> Thanks,
>
> Simon
>
Regards,
Ruslan
On 2017-07-26 12:49, Ruslan Kabatsayev wrote:
>>> @@ -2280,9 +2289,15 @@ default_print_one_register_info (struct
>>> ui_file
>>> *file,
>>> {
>>> struct type *regtype = value_type (val);
>>> int print_raw_format;
>>> + string_file format_stream;
>>> + enum tab_stops
>>> + {
>>> + value_column_1=15,
>>> + value_column_2=32,
>>
>> Perhaps those could have more explicit names, like "column_hex" and
>> "column_natural"?
> This was my first thought, but then I noticed that for floating-point
> registers we have "(raw 0x....)" on the right column and "1.32536e+52"
> on the left one. Do you still think it's OK to name the left column hex
> and the right one natural?
Ah I see. Well that seems inconsistent, I wonder if there's a reason
it's this way. In that case, I'm fine with the names.
Simon
@@ -2270,6 +2270,15 @@ path_command (char *dirname, int from_tty)
}
+static void
+pad_to_column_prepending_tab (string_file& stream, int col)
+{
+ const int size_with_tab = stream.size () / 8 * 8 + 8;
+ stream.putc ('\t');
+ if (size_with_tab < col)
+ stream.puts (n_spaces (col - size_with_tab));
+}
+
/* Print out the register NAME with value VAL, to FILE, in the default
fashion. */
@@ -2280,9 +2289,15 @@ default_print_one_register_info (struct ui_file *file,
{
struct type *regtype = value_type (val);
int print_raw_format;
+ string_file format_stream;
+ enum tab_stops
+ {
+ value_column_1=15,
+ value_column_2=32,
+ };
- fputs_filtered (name, file);
- print_spaces_filtered (15 - strlen (name), file);
+ format_stream.puts (name);
+ format_stream.puts (n_spaces (value_column_1 - strlen (name)));
print_raw_format = (value_entirely_available (val)
&& !value_optimized_out (val));
@@ -2301,14 +2316,15 @@ default_print_one_register_info (struct ui_file *file,
val_print (regtype,
value_embedded_offset (val), 0,
- file, 0, val, &opts, current_language);
+ &format_stream, 0, val, &opts, current_language);
if (print_raw_format)
{
- fprintf_filtered (file, "\t(raw ");
- print_hex_chars (file, valaddr, TYPE_LENGTH (regtype), byte_order,
+ pad_to_column_prepending_tab (format_stream, value_column_2);
+ format_stream.puts ("(raw ");
+ print_hex_chars (&format_stream, valaddr, TYPE_LENGTH (regtype), byte_order,
true);
- fprintf_filtered (file, ")");
+ format_stream.puts (")");
}
}
else
@@ -2320,20 +2336,21 @@ default_print_one_register_info (struct ui_file *file,
opts.deref_ref = 1;
val_print (regtype,
value_embedded_offset (val), 0,
- file, 0, val, &opts, current_language);
+ &format_stream, 0, val, &opts, current_language);
/* If not a vector register, print it also according to its
natural format. */
if (print_raw_format && TYPE_VECTOR (regtype) == 0)
{
+ pad_to_column_prepending_tab (format_stream, value_column_2);
get_user_print_options (&opts);
opts.deref_ref = 1;
- fprintf_filtered (file, "\t");
val_print (regtype,
value_embedded_offset (val), 0,
- file, 0, val, &opts, current_language);
+ &format_stream, 0, val, &opts, current_language);
}
}
+ fputs_filtered (format_stream.c_str (), file);
fprintf_filtered (file, "\n");
}