[v2,1/3] Add zero-padded hexadecimal format support for varobj's

Message ID 554CFF15.1090206@codesourcery.com
State New, archived
Headers

Commit Message

Luis Machado May 8, 2015, 6:23 p.m. UTC
  On 05/08/2015 02:28 PM, Joel Brobecker wrote:
>> gdb/ChangeLog:
>>
>> 2015-05-07  Luis Machado  <lgustavo@codesourcery.com>
>>
>> 	* gdb/mi/mi-cmd-var.c (mi_parse_format): Handle new "zero-hexadecimal"
>> 	format.
>> 	* gdb/varobj.c (varobj_format_string): Add "zero-hexadecimal" entry.
>> 	(format_code): Add 'z' entry.
>> 	(varobj_set_display_format): Handle FORMAT_ZHEXADECIMAL.
>> 	* gdb/varobj.h (varobj_display_formats) <FORMAT_ZHEXADECIMAL>: New enum
>> 	field.
>
> I don't normally have authority to approve, but the patch looks
> sufficiently mechanical that I think I can provide approval if
> the area maintainer isn't available.
>
> In the meantime, I noticed a few nits (formatting, mostly).
>
>>     error (_("Must specify the format as: \"natural\", "
>> -	   "\"binary\", \"decimal\", \"hexadecimal\", or \"octal\""));
>> +	   "\"binary\", \"decimal\", \"hexadecimal\", \"octal\" or \"zero-hexadecimal\""));
>
> The last line is too long. Can you split it?
>
>> @@ -50,7 +50,7 @@ show_varobjdebug (struct ui_file *file, int from_tty,
>>
>>   /* String representations of gdb's format codes.  */
>>   char *varobj_format_string[] =
>> -  { "natural", "binary", "decimal", "hexadecimal", "octal" };
>> +  { "natural", "binary", "decimal", "hexadecimal", "octal", "zero-hexadecimal" };
>
> Same here.
>
>> --- a/gdb/varobj.h
>> +++ b/gdb/varobj.h
>> @@ -28,7 +28,8 @@ enum varobj_display_formats
>>       FORMAT_BINARY,		/* Binary display                    */
>>       FORMAT_DECIMAL,		/* Decimal display                   */
>>       FORMAT_HEXADECIMAL,		/* Hex display                       */
>> -    FORMAT_OCTAL		/* Octal display                     */
>> +    FORMAT_OCTAL,		/* Octal display                     */
>> +    FORMAT_ZHEXADECIMAL		/* Zero padded hexadecimal	     */
>>     };
>
> I suggest adding a ',' at the end of FORMAT_ZHEXADECIMAL.
> That way, next time we add a new enum, we can just add it
> without touching the rest of the definition.
>

Thanks Joel.

I've updated the patch with your suggestions now.

Luis
  

Comments

Joel Brobecker May 11, 2015, 8:55 p.m. UTC | #1
> On 05/08/2015 02:28 PM, Joel Brobecker wrote:
> >>gdb/ChangeLog:
> >>
> >>2015-05-07  Luis Machado  <lgustavo@codesourcery.com>
> >>
> >>	* gdb/mi/mi-cmd-var.c (mi_parse_format): Handle new "zero-hexadecimal"
> >>	format.
> >>	* gdb/varobj.c (varobj_format_string): Add "zero-hexadecimal" entry.
> >>	(format_code): Add 'z' entry.
> >>	(varobj_set_display_format): Handle FORMAT_ZHEXADECIMAL.
> >>	* gdb/varobj.h (varobj_display_formats) <FORMAT_ZHEXADECIMAL>: New enum
> >>	field.
> >
> >I don't normally have authority to approve, but the patch looks
> >sufficiently mechanical that I think I can provide approval if
> >the area maintainer isn't available.
[...]
> I've updated the patch with your suggestions now.

Thanks, Luis. FWIW, the patch looks good to me. Let's wait one more
week for the MI maintainer to give any feedback, but otherwise just
go ahead and push (based on the fact that it's fairly consistent with
what we already have, and a fairly mechanical change).
  
Luis Machado May 20, 2015, 12:15 p.m. UTC | #2
On 05/11/2015 05:55 PM, Joel Brobecker wrote:
>> On 05/08/2015 02:28 PM, Joel Brobecker wrote:
>>>> gdb/ChangeLog:
>>>>
>>>> 2015-05-07  Luis Machado  <lgustavo@codesourcery.com>
>>>>
>>>> 	* gdb/mi/mi-cmd-var.c (mi_parse_format): Handle new "zero-hexadecimal"
>>>> 	format.
>>>> 	* gdb/varobj.c (varobj_format_string): Add "zero-hexadecimal" entry.
>>>> 	(format_code): Add 'z' entry.
>>>> 	(varobj_set_display_format): Handle FORMAT_ZHEXADECIMAL.
>>>> 	* gdb/varobj.h (varobj_display_formats) <FORMAT_ZHEXADECIMAL>: New enum
>>>> 	field.
>>>
>>> I don't normally have authority to approve, but the patch looks
>>> sufficiently mechanical that I think I can provide approval if
>>> the area maintainer isn't available.
> [...]
>> I've updated the patch with your suggestions now.
>
> Thanks, Luis. FWIW, the patch looks good to me. Let's wait one more
> week for the MI maintainer to give any feedback, but otherwise just
> go ahead and push (based on the fact that it's fairly consistent with
> what we already have, and a fairly mechanical change).
>

Thanks Joel. I'll push this one until the end of the week.
  
Luis Machado May 25, 2015, 11:39 p.m. UTC | #3
On 05/20/2015 09:15 AM, Luis Machado wrote:
> On 05/11/2015 05:55 PM, Joel Brobecker wrote:
>>> On 05/08/2015 02:28 PM, Joel Brobecker wrote:
>>>>> gdb/ChangeLog:
>>>>>
>>>>> 2015-05-07  Luis Machado  <lgustavo@codesourcery.com>
>>>>>
>>>>>     * gdb/mi/mi-cmd-var.c (mi_parse_format): Handle new
>>>>> "zero-hexadecimal"
>>>>>     format.
>>>>>     * gdb/varobj.c (varobj_format_string): Add "zero-hexadecimal"
>>>>> entry.
>>>>>     (format_code): Add 'z' entry.
>>>>>     (varobj_set_display_format): Handle FORMAT_ZHEXADECIMAL.
>>>>>     * gdb/varobj.h (varobj_display_formats) <FORMAT_ZHEXADECIMAL>:
>>>>> New enum
>>>>>     field.
>>>>
>>>> I don't normally have authority to approve, but the patch looks
>>>> sufficiently mechanical that I think I can provide approval if
>>>> the area maintainer isn't available.
>> [...]
>>> I've updated the patch with your suggestions now.
>>
>> Thanks, Luis. FWIW, the patch looks good to me. Let's wait one more
>> week for the MI maintainer to give any feedback, but otherwise just
>> go ahead and push (based on the fact that it's fairly consistent with
>> what we already have, and a fairly mechanical change).
>>
>
> Thanks Joel. I'll push this one until the end of the week.
>
>

I've pushed this now.
  

Patch

gdb/ChangeLog:

2015-05-08  Luis Machado  <lgustavo@codesourcery.com>

	* gdb/mi/mi-cmd-var.c (mi_parse_format): Handle new
	"zero-hexadecimal" format.
	* gdb/varobj.c (varobj_format_string): Add "zero-hexadecimal"
	entry.
	(format_code): Add 'z' entry.
	(varobj_set_display_format): Handle FORMAT_ZHEXADECIMAL.
	* gdb/varobj.h (varobj_display_formats) <FORMAT_ZHEXADECIMAL>: New
	enum field.
---
 gdb/mi/mi-cmd-var.c | 5 ++++-
 gdb/varobj.c        | 6 ++++--
 gdb/varobj.h        | 3 ++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index ee0bbc6..fcf3060 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -233,10 +233,13 @@  mi_parse_format (const char *arg)
 	return FORMAT_HEXADECIMAL;
       else if (strncmp (arg, "octal", len) == 0)
 	return FORMAT_OCTAL;
+      else if (strncmp (arg, "zero-hexadecimal", len) == 0)
+	return FORMAT_ZHEXADECIMAL;
     }
 
   error (_("Must specify the format as: \"natural\", "
-	   "\"binary\", \"decimal\", \"hexadecimal\", or \"octal\""));
+	   "\"binary\", \"decimal\", \"hexadecimal\", \"octal\" or "
+	   "\"zero-hexadecimal\""));
 }
 
 void
diff --git a/gdb/varobj.c b/gdb/varobj.c
index b220fd8..87c48a4 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -50,7 +50,8 @@  show_varobjdebug (struct ui_file *file, int from_tty,
 
 /* String representations of gdb's format codes.  */
 char *varobj_format_string[] =
-  { "natural", "binary", "decimal", "hexadecimal", "octal" };
+  { "natural", "binary", "decimal", "hexadecimal", "octal",
+    "zero-hexadecimal" };
 
 /* True if we want to allow Python-based pretty-printing.  */
 static int pretty_printing = 0;
@@ -214,7 +215,7 @@  static struct varobj *varobj_add_child (struct varobj *var,
 /* Private data */
 
 /* Mappings of varobj_display_formats enums to gdb's format codes.  */
-static int format_code[] = { 0, 't', 'd', 'x', 'o' };
+static int format_code[] = { 0, 't', 'd', 'x', 'o', 'z' };
 
 /* Header of the list of root variable objects.  */
 static struct varobj_root *rootlist;
@@ -583,6 +584,7 @@  varobj_set_display_format (struct varobj *var,
     case FORMAT_DECIMAL:
     case FORMAT_HEXADECIMAL:
     case FORMAT_OCTAL:
+    case FORMAT_ZHEXADECIMAL:
       var->format = format;
       break;
 
diff --git a/gdb/varobj.h b/gdb/varobj.h
index 8860526..0f643aa 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -28,7 +28,8 @@  enum varobj_display_formats
     FORMAT_BINARY,		/* Binary display                    */
     FORMAT_DECIMAL,		/* Decimal display                   */
     FORMAT_HEXADECIMAL,		/* Hex display                       */
-    FORMAT_OCTAL		/* Octal display                     */
+    FORMAT_OCTAL,		/* Octal display                     */
+    FORMAT_ZHEXADECIMAL,	/* Zero-padded hexadecimal	     */
   };
 
 enum varobj_type
-- 
1.9.1