[PATCH] [1/2] Add new 'z' format for print command

[PATCH] [1/2] Add new 'z' format for print command

[Andrew Burgess]

The MI code for displaying registers currently supports
a special case 'r' format that displays the register in
'raw' form, this is hexadecimal, zero padded to the size
of the register.

Having this as a special case in the MI code causes problems
with optimized out registers, which I tried addressing in a
patch here:
  http://sourceware.org/ml/gdb-patches/2013-07/msg00193.html

Pedro pointed out that I could do better, so, this patch
introduces a new 'z' format within the core value printing
code that displays scalars using zero padded hexadecimal.
I've mentioned this in the docs and added a test for the
new format.

In patch [2/2] I'll change the MI code to make use of this
new 'z' formatter.

OK to apply?

Thanks,
Andrew



gdb/ChangeLog

2013-07-08  Andrew Burgess  <aburgess@broadcom.com>

	* printcmd.c (print_scalar_formatted): Add new 'z' formatter.

gdb/docs/ChangeLog

2013-07-08  Andrew Burgess  <aburgess@broadcom.com>

	* gdb.texinfo (Output Formats): Mention the new 'z' formatter.

gdb/testsuite/ChangeLog

2013-07-08  Andrew Burgess  <aburgess@broadcom.com>

	* gdb.base/printcmds.exp (test_print_int_arrays): Add tests for x,
	z, o, and t output formats.
	* gdb.base/display.exp: Use 'k' as an undefined format now that
	'z' is defined.

 
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 99d4dba..f34d18b 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -533,6 +533,10 @@ print_scalar_formatted (const void *valaddr, struct type *type,
       }
       break;
 
+    case 'z':
+      print_hex_chars (stream, valaddr, len, byte_order);
+      break;
+
     default:
       error (_("Undefined output format \"%c\"."), options->format);
     }
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index fae54e4..c5d7754 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -8517,6 +8517,11 @@ Without this format, @value{GDBN} displays pointers to and arrays of
 strings.  Single-byte members of a vector are displayed as an integer
 array.
 
+@item z
+Like @samp{x} formatting, the integer is printed as hexadecimal, but
+leading zeros are printed to pad the value to the size of the integer
+type.
+
 @item r
 @cindex raw printing
 Print using the @samp{raw} formatting.  By default, @value{GDBN} will
diff --git a/gdb/testsuite/gdb.base/display.exp b/gdb/testsuite/gdb.base/display.exp
index 5db84cc..9a0b1dd 100644
--- a/gdb/testsuite/gdb.base/display.exp
+++ b/gdb/testsuite/gdb.base/display.exp
@@ -168,7 +168,7 @@ gdb_test "printf \"%p\\n\", 1" "0x1"
 
 # play with "print", too
 #
-gdb_test "print/z j" ".*Undefined output format.*"
+gdb_test "print/k j" ".*Undefined output format.*"
 gdb_test "print/d j" " = 0\[\\r\\n\]+"   "debug test output 1"
 gdb_test "print/r j" " = 0\[\\r\\n\]+"   "debug test output 1a"
 gdb_test "print/x j" " = 0x0\[\\r\\n\]+" "debug test output 2"
diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
index 0597ab0..4883fd5 100644
--- a/gdb/testsuite/gdb.base/printcmds.exp
+++ b/gdb/testsuite/gdb.base/printcmds.exp
@@ -587,6 +587,16 @@ proc test_print_int_arrays {} {
 	" = {{{0, 1}, {2, 3}, {4, 5}}, {{6, 7}, {8, 9}, {10, 11}}}"
     gdb_test_escape_braces "p int4dim" \
 	" = {{{{0, 1}, {2, 3}, {4, 5}}, {{6, 7}, {8, 9}, {10, 11}}}}"
+
+    # Some checks for various output formats.
+    gdb_test_escape_braces "p/x int4dim" \
+	" = {{{{0x0, 0x1}, {0x2, 0x3}, {0x4, 0x5}}, {{0x6, 0x7}, {0x8, 0x9}, {0xa, 0xb}}}}"
+    gdb_test_escape_braces "p/z int4dim" \
+       " = {{{{0x0+0, 0x0+1}, {0x0+2, 0x0+3}, {0x0+4, 0x0+5}}, {{0x0+6, 0x0+7}, {0x0+8, 0x0+9}, {0x0+a, 0x0+b}}}}"
+    gdb_test_escape_braces "p/o int4dim" \
+       " = {{{{0, 01}, {02, 03}, {04, 05}}, {{06, 07}, {010, 011}, {012, 013}}}}"
+    gdb_test_escape_braces "p/t int4dim" \
+       " = {{{{0, 1}, {10, 11}, {100, 101}}, {{110, 111}, {1000, 1001}, {1010, 1011}}}}"
 }
 
 proc test_print_typedef_arrays {} {

Re: [PATCH] [1/2] Add new 'z' format for print command

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@broadcom.com> writes:

Andrew> Pedro pointed out that I could do better, so, this patch
Andrew> introduces a new 'z' format within the core value printing
Andrew> code that displays scalars using zero padded hexadecimal.
Andrew> I've mentioned this in the docs and added a test for the
Andrew> new format.

Andrew> In patch [2/2] I'll change the MI code to make use of this
Andrew> new 'z' formatter.

Andrew> OK to apply?

It looks good to me.  However, it needs a doc review, and also, I think,
a NEWS entry.

Plus, I think we should wait a little bit in case anyone has an issue
with the choice of 'z'.

thanks,
Tom

Re: [PATCH] [1/2] Add new 'z' format for print command

[Andrew Burgess]

On 10/07/2013 6:30 PM, Tom Tromey wrote:
>>>>>> "Andrew" == Andrew Burgess <aburgess@broadcom.com> writes:
> 
> Andrew> Pedro pointed out that I could do better, so, this patch
> Andrew> introduces a new 'z' format within the core value printing
> Andrew> code that displays scalars using zero padded hexadecimal.
> Andrew> I've mentioned this in the docs and added a test for the
> Andrew> new format.
> 
> Andrew> In patch [2/2] I'll change the MI code to make use of this
> Andrew> new 'z' formatter.
> 
> Andrew> OK to apply?
> 
> It looks good to me.  However, it needs a doc review, and also, I think,
> a NEWS entry.

Thanks for looking through this patch.

I've included a proposed NEWS entry below.

It also occurred to me that the 'z' formatter should be mentioned in the
help text for the 'x' command, I've included a small patch below that
makes this change.

> Plus, I think we should wait a little bit in case anyone has an issue
> with the choice of 'z'.

I'll wait a while to see if there's any feedback on the choice of
letter, and for a docs review.

Thanks,
Andrew


gdb/NEWS

* New 'z' formatter for print and examine memory commands, displays the
  value as hexadecimal zero padded to the size of the type.

gdb/ChangeLog

2013-07-08  Andrew Burgess  <aburgess@broadcom.com>

	* printcmd.c (_initialize_printcmd): Mention 'z' formatter in
	help text of the 'x' command.

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index f34d18b..0862f86 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2500,7 +2500,8 @@ Examine memory: x/FMT ADDRESS.\n\
 ADDRESS is an expression for the memory address to examine.\n\
 FMT is a repeat count followed by a format letter and a size letter.\n\
 Format letters are o(octal), x(hex), d(decimal), u(unsigned decimal),\n\
-  t(binary), f(float), a(address), i(instruction), c(char) and s(string).\n\
+  t(binary), f(float), a(address), i(instruction), c(char), s(string)\n\
+  and z(zero padded hex).\n\
 Size letters are b(byte), h(halfword), w(word), g(giant, 8 bytes).\n\
 The specified number of objects of the specified size are printed\n\
 according to the format.\n\n\

Re: [PATCH] [1/2] Add new 'z' format for print command

[Andrew Burgess]

On 11/07/2013 4:20 PM, Andrew Burgess wrote:
> On 10/07/2013 6:30 PM, Tom Tromey wrote:
>>>>>>> "Andrew" == Andrew Burgess <aburgess@broadcom.com> writes:
>>
>> Andrew> Pedro pointed out that I could do better, so, this patch
>> Andrew> introduces a new 'z' format within the core value printing
>> Andrew> code that displays scalars using zero padded hexadecimal.
>> Andrew> I've mentioned this in the docs and added a test for the
>> Andrew> new format.
>>
>> Andrew> In patch [2/2] I'll change the MI code to make use of this
>> Andrew> new 'z' formatter.
>>
>> Andrew> OK to apply?
>>
>> It looks good to me.  However, it needs a doc review, and also, I think,
>> a NEWS entry.

Ping.

>> Plus, I think we should wait a little bit in case anyone has an issue
>> with the choice of 'z'.

As nobody has objected yet I'm going to assume that the use of 'z' is ok....

Thanks,
Andrew

[PING (docs)] Re: [PATCH] [1/2] Add new 'z' format for print command

[Andrew Burgess]

I just need a docs review on the change below please.

Following Toms feedback I followed up with a NEWS entry in
this post:
  http://sourceware.org/ml/gdb-patches/2013-07/msg00319.html

I've included these extra changes at the end of this mail
to make reviewing easier.

Thanks,
Andrew


On 08/07/2013 6:30 PM, Andrew Burgess wrote:
> The MI code for displaying registers currently supports
> a special case 'r' format that displays the register in
> 'raw' form, this is hexadecimal, zero padded to the size
> of the register.
> 
> Having this as a special case in the MI code causes problems
> with optimized out registers, which I tried addressing in a
> patch here:
>   http://sourceware.org/ml/gdb-patches/2013-07/msg00193.html
> 
> Pedro pointed out that I could do better, so, this patch
> introduces a new 'z' format within the core value printing
> code that displays scalars using zero padded hexadecimal.
> I've mentioned this in the docs and added a test for the
> new format.
> 
> In patch [2/2] I'll change the MI code to make use of this
> new 'z' formatter.
> 
> OK to apply?
> 
> Thanks,
> Andrew
> 
> 
> 
> gdb/ChangeLog
> 
> 2013-07-08  Andrew Burgess  <aburgess@broadcom.com>
> 
> 	* printcmd.c (print_scalar_formatted): Add new 'z' formatter.
> 
> gdb/docs/ChangeLog
> 
> 2013-07-08  Andrew Burgess  <aburgess@broadcom.com>
> 
> 	* gdb.texinfo (Output Formats): Mention the new 'z' formatter.
> 
> gdb/testsuite/ChangeLog
> 
> 2013-07-08  Andrew Burgess  <aburgess@broadcom.com>
> 
> 	* gdb.base/printcmds.exp (test_print_int_arrays): Add tests for x,
> 	z, o, and t output formats.
> 	* gdb.base/display.exp: Use 'k' as an undefined format now that
> 	'z' is defined.
> 
>  
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index 99d4dba..f34d18b 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -533,6 +533,10 @@ print_scalar_formatted (const void *valaddr, struct type *type,
>        }
>        break;
>  
> +    case 'z':
> +      print_hex_chars (stream, valaddr, len, byte_order);
> +      break;
> +
>      default:
>        error (_("Undefined output format \"%c\"."), options->format);
>      }
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index fae54e4..c5d7754 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -8517,6 +8517,11 @@ Without this format, @value{GDBN} displays pointers to and arrays of
>  strings.  Single-byte members of a vector are displayed as an integer
>  array.
>  
> +@item z
> +Like @samp{x} formatting, the integer is printed as hexadecimal, but
> +leading zeros are printed to pad the value to the size of the integer
> +type.
> +
>  @item r
>  @cindex raw printing
>  Print using the @samp{raw} formatting.  By default, @value{GDBN} will
> diff --git a/gdb/testsuite/gdb.base/display.exp b/gdb/testsuite/gdb.base/display.exp
> index 5db84cc..9a0b1dd 100644
> --- a/gdb/testsuite/gdb.base/display.exp
> +++ b/gdb/testsuite/gdb.base/display.exp
> @@ -168,7 +168,7 @@ gdb_test "printf \"%p\\n\", 1" "0x1"
>  
>  # play with "print", too
>  #
> -gdb_test "print/z j" ".*Undefined output format.*"
> +gdb_test "print/k j" ".*Undefined output format.*"
>  gdb_test "print/d j" " = 0\[\\r\\n\]+"   "debug test output 1"
>  gdb_test "print/r j" " = 0\[\\r\\n\]+"   "debug test output 1a"
>  gdb_test "print/x j" " = 0x0\[\\r\\n\]+" "debug test output 2"
> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
> index 0597ab0..4883fd5 100644
> --- a/gdb/testsuite/gdb.base/printcmds.exp
> +++ b/gdb/testsuite/gdb.base/printcmds.exp
> @@ -587,6 +587,16 @@ proc test_print_int_arrays {} {
>  	" = {{{0, 1}, {2, 3}, {4, 5}}, {{6, 7}, {8, 9}, {10, 11}}}"
>      gdb_test_escape_braces "p int4dim" \
>  	" = {{{{0, 1}, {2, 3}, {4, 5}}, {{6, 7}, {8, 9}, {10, 11}}}}"
> +
> +    # Some checks for various output formats.
> +    gdb_test_escape_braces "p/x int4dim" \
> +	" = {{{{0x0, 0x1}, {0x2, 0x3}, {0x4, 0x5}}, {{0x6, 0x7}, {0x8, 0x9}, {0xa, 0xb}}}}"
> +    gdb_test_escape_braces "p/z int4dim" \
> +       " = {{{{0x0+0, 0x0+1}, {0x0+2, 0x0+3}, {0x0+4, 0x0+5}}, {{0x0+6, 0x0+7}, {0x0+8, 0x0+9}, {0x0+a, 0x0+b}}}}"
> +    gdb_test_escape_braces "p/o int4dim" \
> +       " = {{{{0, 01}, {02, 03}, {04, 05}}, {{06, 07}, {010, 011}, {012, 013}}}}"
> +    gdb_test_escape_braces "p/t int4dim" \
> +       " = {{{{0, 1}, {10, 11}, {100, 101}}, {{110, 111}, {1000, 1001}, {1010, 1011}}}}"
>  }
>  
>  proc test_print_typedef_arrays {} {
> 

Additional NEWS entry, and a small extra patch to mention this feature
in the inline help:

gdb/NEWS

* New 'z' formatter for print and examine memory commands, displays the
  value as hexadecimal zero padded to the size of the type.
 
gdb/ChangeLog

2013-07-08  Andrew Burgess  <aburgess@broadcom.com>
 
	* printcmd.c (_initialize_printcmd): Mention 'z' formatter in
	help text of the 'x' command.
	* NEWS: Mention the new 'z' formatter.

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index f34d18b..0862f86 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2500,7 +2500,8 @@ Examine memory: x/FMT ADDRESS.\n\
 ADDRESS is an expression for the memory address to examine.\n\
 FMT is a repeat count followed by a format letter and a size letter.\n\
 Format letters are o(octal), x(hex), d(decimal), u(unsigned decimal),\n\
-  t(binary), f(float), a(address), i(instruction), c(char) and s(string).\n\
+  t(binary), f(float), a(address), i(instruction), c(char), s(string)\n\
+  and z(zero padded hex).\n\
 Size letters are b(byte), h(halfword), w(word), g(giant, 8 bytes).\n\
 The specified number of objects of the specified size are printed\n\
 according to the format.\n\n\

Re: [PING (docs)] Re: [PATCH] [1/2] Add new 'z' format for print command

[Eli Zaretskii]

> Date: Wed, 24 Jul 2013 11:07:12 +0100
> From: "Andrew Burgess" <aburgess@broadcom.com>
> cc: "Eli Zaretskii" <eliz@gnu.org>
> 
> I just need a docs review on the change below please.

Sorry for missing it.

> gdb/NEWS
> 
> * New 'z' formatter for print and examine memory commands, displays the
>   value as hexadecimal zero padded to the size of the type.

The comma before "displays" should be removed.  Also, "for printing
and examining", rather than "print and examine".

More importantly, I have a difficulty understanding what does
"hexadecimal zero padded to the size ..." mean.  More accurately, if
my interpretation of what you say (that 'z' produces "00"s for
non-existing data, then why does this "padding" make sense, when we
are targeting a human consumer?

Thanks.

Re: [PING (docs)] Re: [PATCH] [1/2] Add new 'z' format for print command

[Andrew Burgess]

On 24/07/2013 3:34 PM, Eli Zaretskii wrote:

> More importantly, I have a difficulty understanding what does
> "hexadecimal zero padded to the size ..." mean.  More accurately, if
> my interpretation of what you say (that 'z' produces "00"s for
> non-existing data, then why does this "padding" make sense, when we
> are targeting a human consumer?

I'm not sure I agree with the "non-existing data" part of your
statement, for example a 4-byte register containing the value 1 (one)
when printed with /x would print 0x1, with /z it would print 0x00000001,
surely the leading 0s do exist, we just normally don't print them.

I agree that for a human consumer /z might not be so useful, but moving
this code from being MI specific, to being a core format has cleaned up
an annoying bug, and does mean that the two interfaces offer the same
set of display formats.

You might argue that either:

  (1) We should prevent this format being used in non-mi mode as it is
      not useful.  Given what a small feature this is, and for the
      consistent interface reason, I'd prefer to keep it available. Or,

  (2) We allow it, but only document it in the MI part of the document,
      so CLI users are not 'tempted' to use it.  I don't like this as it
      presupposes what a user will / will not find useful, and make the
      documentation seem incomplete.

I'd be interested to get your thoughts though.

Thanks,
Andrew

Re: [PING (docs)] Re: [PATCH] [1/2] Add new 'z' format for print command

[Eli Zaretskii]

> Date: Wed, 24 Jul 2013 16:16:01 +0100
> From: "Andrew Burgess" <aburgess@broadcom.com>
> cc: gdb-patches@sourceware.org
> 
> On 24/07/2013 3:34 PM, Eli Zaretskii wrote:
> 
> > More importantly, I have a difficulty understanding what does
> > "hexadecimal zero padded to the size ..." mean.  More accurately, if
> > my interpretation of what you say (that 'z' produces "00"s for
> > non-existing data, then why does this "padding" make sense, when we
> > are targeting a human consumer?
> 
> I'm not sure I agree with the "non-existing data" part of your
> statement, for example a 4-byte register containing the value 1 (one)
> when printed with /x would print 0x1, with /z it would print 0x00000001,
> surely the leading 0s do exist, we just normally don't print them.

Then please say "zero padded on the left".  I thought you were talking
about padding at the right.  Sorry for my misunderstanding.

Re: [PING (docs)] Re: [PATCH] [1/2] Add new 'z' format for print command

[Andrew Burgess]

On 24/07/2013 6:00 PM, Eli Zaretskii wrote:
>> Date: Wed, 24 Jul 2013 16:16:01 +0100
>> From: "Andrew Burgess" <aburgess@broadcom.com>
>> cc: gdb-patches@sourceware.org
>>
>> On 24/07/2013 3:34 PM, Eli Zaretskii wrote:
>>
>>> More importantly, I have a difficulty understanding what does
>>> "hexadecimal zero padded to the size ..." mean.  More accurately, if
>>> my interpretation of what you say (that 'z' produces "00"s for
>>> non-existing data, then why does this "padding" make sense, when we
>>> are targeting a human consumer?
>>
>> I'm not sure I agree with the "non-existing data" part of your
>> statement, for example a 4-byte register containing the value 1 (one)
>> when printed with /x would print 0x1, with /z it would print 0x00000001,
>> surely the leading 0s do exist, we just normally don't print them.
> 
> Then please say "zero padded on the left".  I thought you were talking
> about padding at the right.  Sorry for my misunderstanding.

OK.  I'll re-word that tomorrow, do you want to see a revised patch
before I commit?

Thanks for your time,
Andrew

Re: [PING (docs)] Re: [PATCH] [1/2] Add new 'z' format for print command

[Eli Zaretskii]

> Date: Wed, 24 Jul 2013 18:08:22 +0100
> From: "Andrew Burgess" <aburgess@broadcom.com>
> cc: gdb-patches@sourceware.org
> 
> > Then please say "zero padded on the left".  I thought you were talking
> > about padding at the right.  Sorry for my misunderstanding.
> 
> OK.  I'll re-word that tomorrow, do you want to see a revised patch
> before I commit?

No need, you can commit the fixed text.

Re: [PING (docs)] Re: [PATCH] [1/2] Add new 'z' format for print command

[Andrew Burgess]

On 24/07/2013 6:42 PM, Eli Zaretskii wrote:
>> Date: Wed, 24 Jul 2013 18:08:22 +0100
>> From: "Andrew Burgess" <aburgess@broadcom.com>
>> cc: gdb-patches@sourceware.org
>>
>>> Then please say "zero padded on the left".  I thought you were talking
>>> about padding at the right.  Sorry for my misunderstanding.
>>
>> OK.  I'll re-word that tomorrow, do you want to see a revised patch
>> before I commit?
> 
> No need, you can commit the fixed text.

Committed with changes suggested.

Thanks,
Andrew