diff mbox

[2/4] x86-dregs: Print debug registers one per line

Message ID 1498076108-29914-3-git-send-email-simon.marchi@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi June 21, 2017, 8:15 p.m. UTC 
  This get around this warning given by clang...

  /home/emaisin/src/binutils-gdb/gdb/nat/x86-dregs.c:209:7: error: variable 'i' is incremented both in the loop header and in the loop body [-Werror,-Wfor-loop-analysis]
        i++;
        ^
  /home/emaisin/src/binutils-gdb/gdb/nat/x86-dregs.c:199:32: note: incremented here
    ALL_DEBUG_ADDRESS_REGISTERS (i)
                               ^

... I decided in the end to simply print the debug registers one per
line.  I don't think it particularly helps readability to have them two
per line anyway.

gdb/ChangeLog:

	* nat/x86-dregs.c (x86_show_dr): Print registers one per line.
---
 gdb/nat/x86-dregs.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

Comments

Sergio Durigan Junior June 21, 2017, 8:31 p.m. UTC | #1 
On Wednesday, June 21 2017, Simon Marchi wrote:

> This get around this warning given by clang...
>
>   /home/emaisin/src/binutils-gdb/gdb/nat/x86-dregs.c:209:7: error: variable 'i' is incremented both in the loop header and in the loop body [-Werror,-Wfor-loop-analysis]
>         i++;
>         ^
>   /home/emaisin/src/binutils-gdb/gdb/nat/x86-dregs.c:199:32: note: incremented here
>     ALL_DEBUG_ADDRESS_REGISTERS (i)
>                                ^
>
> ... I decided in the end to simply print the debug registers one per
> line.  I don't think it particularly helps readability to have them two
> per line anyway.

Agreed, one per line sounds better to me.

>
> gdb/ChangeLog:
>
> 	* nat/x86-dregs.c (x86_show_dr): Print registers one per line.
> ---
>  gdb/nat/x86-dregs.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/nat/x86-dregs.c b/gdb/nat/x86-dregs.c
> index 8c8adfa..58b1179 100644
> --- a/gdb/nat/x86-dregs.c
> +++ b/gdb/nat/x86-dregs.c
> @@ -193,20 +193,16 @@ x86_show_dr (struct x86_debug_reg_state *state,
>  			      here.  */
>  			   : "??unknown??"))));
>    debug_printf (":\n");
> -  debug_printf ("\tCONTROL (DR7): %s          STATUS (DR6): %s\n",
> -		phex (state->dr_control_mirror, 8),
> -		phex (state->dr_status_mirror, 8));
> +
> +  debug_printf ("\tCONTROL (DR7): 0x%s\n",phex (state->dr_control_mirror, 8));
                                           ^^^

Space after comma?

> +  debug_printf ("\tSTATUS (DR6): 0x%s\n", phex (state->dr_status_mirror, 8));
> +
>    ALL_DEBUG_ADDRESS_REGISTERS (i)
>      {
> -      debug_printf ("\
> -\tDR%d: addr=0x%s, ref.count=%d  DR%d: addr=0x%s, ref.count=%d\n",
> +      debug_printf ("\tDR%d: addr=0x%s, ref.count=%d\n",
>  		    i, phex (state->dr_mirror[i],
>  			     x86_get_debug_register_length ()),
> -		    state->dr_ref_count[i],
> -		    i + 1, phex (state->dr_mirror[i + 1],
> -				 x86_get_debug_register_length ()),
> -		    state->dr_ref_count[i + 1]);
> -      i++;
> +		    state->dr_ref_count[i]);
>      }
>  }
>  
> -- 
> 2.7.4

LGTM.  Thanks,
Simon Marchi June 21, 2017, 9:06 p.m. UTC | #2 
On 2017-06-21 22:31, Sergio Durigan Junior wrote:
> On Wednesday, June 21 2017, Simon Marchi wrote:
> 
>> This get around this warning given by clang...
>> 
>>   /home/emaisin/src/binutils-gdb/gdb/nat/x86-dregs.c:209:7: error: 
>> variable 'i' is incremented both in the loop header and in the loop 
>> body [-Werror,-Wfor-loop-analysis]
>>         i++;
>>         ^
>>   /home/emaisin/src/binutils-gdb/gdb/nat/x86-dregs.c:199:32: note: 
>> incremented here
>>     ALL_DEBUG_ADDRESS_REGISTERS (i)
>>                                ^
>> 
>> ... I decided in the end to simply print the debug registers one per
>> line.  I don't think it particularly helps readability to have them 
>> two
>> per line anyway.
> 
> Agreed, one per line sounds better to me.
> 
>> 
>> gdb/ChangeLog:
>> 
>> 	* nat/x86-dregs.c (x86_show_dr): Print registers one per line.
>> ---
>>  gdb/nat/x86-dregs.c | 16 ++++++----------
>>  1 file changed, 6 insertions(+), 10 deletions(-)
>> 
>> diff --git a/gdb/nat/x86-dregs.c b/gdb/nat/x86-dregs.c
>> index 8c8adfa..58b1179 100644
>> --- a/gdb/nat/x86-dregs.c
>> +++ b/gdb/nat/x86-dregs.c
>> @@ -193,20 +193,16 @@ x86_show_dr (struct x86_debug_reg_state *state,
>>  			      here.  */
>>  			   : "??unknown??"))));
>>    debug_printf (":\n");
>> -  debug_printf ("\tCONTROL (DR7): %s          STATUS (DR6): %s\n",
>> -		phex (state->dr_control_mirror, 8),
>> -		phex (state->dr_status_mirror, 8));
>> +
>> +  debug_printf ("\tCONTROL (DR7): 0x%s\n",phex 
>> (state->dr_control_mirror, 8));
>                                            ^^^
> 
> Space after comma?

Yep, thanks.

>> +  debug_printf ("\tSTATUS (DR6): 0x%s\n", phex 
>> (state->dr_status_mirror, 8));
>> +
>>    ALL_DEBUG_ADDRESS_REGISTERS (i)
>>      {
>> -      debug_printf ("\
>> -\tDR%d: addr=0x%s, ref.count=%d  DR%d: addr=0x%s, ref.count=%d\n",
>> +      debug_printf ("\tDR%d: addr=0x%s, ref.count=%d\n",
>>  		    i, phex (state->dr_mirror[i],
>>  			     x86_get_debug_register_length ()),
>> -		    state->dr_ref_count[i],
>> -		    i + 1, phex (state->dr_mirror[i + 1],
>> -				 x86_get_debug_register_length ()),
>> -		    state->dr_ref_count[i + 1]);
>> -      i++;
>> +		    state->dr_ref_count[i]);
>>      }
>>  }
>> 
>> --
>> 2.7.4
> 
> LGTM.  Thanks,

Thanks!
Simon Marchi June 25, 2017, 10:58 a.m. UTC | #3 
On 2017-06-21 23:06, Simon Marchi wrote:
> On 2017-06-21 22:31, Sergio Durigan Junior wrote:
>> On Wednesday, June 21 2017, Simon Marchi wrote:
>> 
>>> This get around this warning given by clang...
>>> 
>>>   /home/emaisin/src/binutils-gdb/gdb/nat/x86-dregs.c:209:7: error: 
>>> variable 'i' is incremented both in the loop header and in the loop 
>>> body [-Werror,-Wfor-loop-analysis]
>>>         i++;
>>>         ^
>>>   /home/emaisin/src/binutils-gdb/gdb/nat/x86-dregs.c:199:32: note: 
>>> incremented here
>>>     ALL_DEBUG_ADDRESS_REGISTERS (i)
>>>                                ^
>>> 
>>> ... I decided in the end to simply print the debug registers one per
>>> line.  I don't think it particularly helps readability to have them 
>>> two
>>> per line anyway.
>> 
>> Agreed, one per line sounds better to me.
>> 
>>> 
>>> gdb/ChangeLog:
>>> 
>>> 	* nat/x86-dregs.c (x86_show_dr): Print registers one per line.
>>> ---
>>>  gdb/nat/x86-dregs.c | 16 ++++++----------
>>>  1 file changed, 6 insertions(+), 10 deletions(-)
>>> 
>>> diff --git a/gdb/nat/x86-dregs.c b/gdb/nat/x86-dregs.c
>>> index 8c8adfa..58b1179 100644
>>> --- a/gdb/nat/x86-dregs.c
>>> +++ b/gdb/nat/x86-dregs.c
>>> @@ -193,20 +193,16 @@ x86_show_dr (struct x86_debug_reg_state *state,
>>>  			      here.  */
>>>  			   : "??unknown??"))));
>>>    debug_printf (":\n");
>>> -  debug_printf ("\tCONTROL (DR7): %s          STATUS (DR6): %s\n",
>>> -		phex (state->dr_control_mirror, 8),
>>> -		phex (state->dr_status_mirror, 8));
>>> +
>>> +  debug_printf ("\tCONTROL (DR7): 0x%s\n",phex 
>>> (state->dr_control_mirror, 8));
>>                                            ^^^
>> 
>> Space after comma?
> 
> Yep, thanks.
> 
>>> +  debug_printf ("\tSTATUS (DR6): 0x%s\n", phex 
>>> (state->dr_status_mirror, 8));
>>> +
>>>    ALL_DEBUG_ADDRESS_REGISTERS (i)
>>>      {
>>> -      debug_printf ("\
>>> -\tDR%d: addr=0x%s, ref.count=%d  DR%d: addr=0x%s, ref.count=%d\n",
>>> +      debug_printf ("\tDR%d: addr=0x%s, ref.count=%d\n",
>>>  		    i, phex (state->dr_mirror[i],
>>>  			     x86_get_debug_register_length ()),
>>> -		    state->dr_ref_count[i],
>>> -		    i + 1, phex (state->dr_mirror[i + 1],
>>> -				 x86_get_debug_register_length ()),
>>> -		    state->dr_ref_count[i + 1]);
>>> -      i++;
>>> +		    state->dr_ref_count[i]);
>>>      }
>>>  }
>>> 
>>> --
>>> 2.7.4
>> 
>> LGTM.  Thanks,
> 
> Thanks!

This is now pushed.
diff mbox

Patch

diff --git a/gdb/nat/x86-dregs.c b/gdb/nat/x86-dregs.c
index 8c8adfa..58b1179 100644
--- a/gdb/nat/x86-dregs.c
+++ b/gdb/nat/x86-dregs.c
@@ -193,20 +193,16 @@  x86_show_dr (struct x86_debug_reg_state *state,
 			      here.  */
 			   : "??unknown??"))));
   debug_printf (":\n");
-  debug_printf ("\tCONTROL (DR7): %s          STATUS (DR6): %s\n",
-		phex (state->dr_control_mirror, 8),
-		phex (state->dr_status_mirror, 8));
+
+  debug_printf ("\tCONTROL (DR7): 0x%s\n",phex (state->dr_control_mirror, 8));
+  debug_printf ("\tSTATUS (DR6): 0x%s\n", phex (state->dr_status_mirror, 8));
+
   ALL_DEBUG_ADDRESS_REGISTERS (i)
     {
-      debug_printf ("\
-\tDR%d: addr=0x%s, ref.count=%d  DR%d: addr=0x%s, ref.count=%d\n",
+      debug_printf ("\tDR%d: addr=0x%s, ref.count=%d\n",
 		    i, phex (state->dr_mirror[i],
 			     x86_get_debug_register_length ()),
-		    state->dr_ref_count[i],
-		    i + 1, phex (state->dr_mirror[i + 1],
-				 x86_get_debug_register_length ()),
-		    state->dr_ref_count[i + 1]);
-      i++;
+		    state->dr_ref_count[i]);
     }
 }