Patchwork [0/5] Remove a few hurdles of compiling with clang

login
register
mail settings
Submitter Pedro Alves
Date June 14, 2017, 10:45 a.m.
Message ID <940cfa4e-b3cb-f15d-93cd-62bd839ab4f0@redhat.com>
Download mbox | patch
Permalink /patch/21005/
State New
Headers show

Comments

Pedro Alves - June 14, 2017, 10:45 a.m.
On 06/14/2017 03:29 AM, Eli Zaretskii wrote:
>> Date: Tue, 13 Jun 2017 22:17:15 +0200
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> Cc: qiyaoltc@gmail.com, gdb-patches@sourceware.org
>>
>> On 2017-06-13 21:22, Eli Zaretskii wrote:
>>> See, I don't consider the proposed solution to be elegant, because it
>>> tweaks a perfectly valid code to placate a stupid compiler warning.
>>
>> It replaces perfectly valid code with some other perfectly valid code.  
> 
> Which is not used anywhere else, right?
> 
>> We can always add comments like
>>
>>    /* Don't use ALL_DEBUG_ADDRESS_REGISTERS here to silence Clang's 
>> -Wfor-loop-analysis warning.  */
> 
> That's the least we should do, IMO.

I'd vote for saying instead:

   /* Don't use ALL_DEBUG_ADDRESS_REGISTERS here because
      we want to skip over two registers at a time.  */

Or better even, just don't skip two registers at a time?
The code is manually printing two columns on each iteration.
How about the patchlet below instead?  I'd call it a clean up on
its own right.

Note this output is only shown with "maint set show-debug-regs on".
So with this, if someone cared for saving vertical space of the
debug output (I don't), this could be easily tweaked to adjust the number
of debug regs per line printed depending on screen width, for example.
Doing that with the current scheme of printing more than
one register with a single format string wouldn't work that well.

From 2cfc65a311798c519ae393f474302f634aa2595d Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 14 Jun 2017 11:25:19 +0100
Subject: [PATCH] dregs

---
 gdb/nat/x86-dregs.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
John Baldwin - June 16, 2017, 4:12 p.m.
On 6/14/17 6:45 AM, Pedro Alves wrote:
> Or better even, just don't skip two registers at a time?
> The code is manually printing two columns on each iteration.
> How about the patchlet below instead?  I'd call it a clean up on
> its own right.

I prefer this fix and agree.  Other places that try to print registers
in columns (mips-tdep.c) iterate one register at a time and make the
newline conditional, so this pattern is consistent with that.

Patch

diff --git a/gdb/nat/x86-dregs.c b/gdb/nat/x86-dregs.c
index 8c8adfa..7fd2d56 100644
--- a/gdb/nat/x86-dregs.c
+++ b/gdb/nat/x86-dregs.c
@@ -198,15 +198,14 @@  x86_show_dr (struct x86_debug_reg_state *state,
 		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",
 		    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]);
+
+      /* Print two debug registers per line.  */
+      if ((i - DR_FIRSTADDR) % 2)
+	debug_printf ("\n");
     }
 }