Re: [PATCH] Fix GCC6 -Wmisleading-indentation issues.

[Mark Wielaard <mjw@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

On Mon, Jan 25, 2016 at 10:24:45PM +0000, Pedro Alves wrote:
> Yeah...  It was part being lazy and port not wanting to mess git blame for
> such old code much.  There's an easy way around it though.  Add an
> else, like:
> 
>   if (tinfo->run_terminal != NULL || gdb_has_a_terminal () == 0)
>     return;
> + else
>     {

O, very nice. I did it that way and adjusted commit message and ChangeLog.

OK to commit?

Thanks,

Mark

[-- Attachment #2: 0001-Fix-GCC6-Wmisleading-indentation-issues.patch --]
[-- Type: text/plain, Size: 7034 bytes --]

From 072fc44cafdff4517fd284da425ac3a8d1ff6343 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Mon, 25 Jan 2016 20:29:54 +0100
Subject: [PATCH] Fix GCC6 -Wmisleading-indentation issues.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

GCC6 will warn about misleading indentation issues like:

gdb/ada-lang.c: In function ‘ada_evaluate_subexp’:
ada-lang.c:11423:9: error: statement is indented as if it were guarded by...
         arg1 = unwrap_value (arg1);
         ^~~~

gdb/ada-lang.c:11421:7: note: ...this ‘else’ clause, but it is not
       else
       ^~~~

In this case it would be a bug except for the fact the if clause already
returned early. So this misindented statement really only got executed
for the else case. But it could easily mislead a reader, so adding a
proper else block is the correct solution.

In case of c-typeprint.c (c_type_print_base) the if statement is indeed
misleadingly indented, but not a bug. Just indent correctly. The inflow.c
(terminal_ours_1) misindented block comes from the removal of an if clause
in commit d9d2d8b which looks correct. Just introduce an else to fixup the
indentation of the block. The linux-record.c misleadingly indented return
statements are just that. Misleading to the reader, but not actual bugs.
Just unindent them so they don't look like they fall under the wrong if
clause.
---
 gdb/ChangeLog      | 11 +++++++++++
 gdb/ada-lang.c     |  8 +++++---
 gdb/c-typeprint.c  | 42 +++++++++++++++++++++---------------------
 gdb/inflow.c       |  2 +-
 gdb/linux-record.c |  8 ++++----
 5 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 476bb5e..8a4453c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2016-01-25  Mark Wielaard  <mjw@redhat.com>
+
+	* ada-lang.c (ada_evaluate_subexp): Add proper else block.
+	* c-typeprint.c (c_type_print_base): Fix misleading indentation of
+	if statement.
+	* inflow.c (child_terminal_ours_1): Fix misleading indentation of
+	statement block by introducing an else.
+	* linux-record.c (record_linux_sockaddr): Fix misleading indentation
+	of return statements.
+	(record_linux_msghdr): Likewise.
+
 2016-01-25  Pedro Alves  <palves@redhat.com>
 
 	PR threads/19461
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 242e632..d874129 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -11419,9 +11419,11 @@ ada_evaluate_subexp (struct type *expect_type, struct expression *exp,
           return value_zero (ada_aligned_type (type), lval_memory);
         }
       else
-        arg1 = ada_value_struct_elt (arg1, &exp->elts[pc + 2].string, 0);
-        arg1 = unwrap_value (arg1);
-        return ada_to_fixed_value (arg1);
+	{
+	  arg1 = ada_value_struct_elt (arg1, &exp->elts[pc + 2].string, 0);
+	  arg1 = unwrap_value (arg1);
+	  return ada_to_fixed_value (arg1);
+	}
 
     case OP_TYPE:
       /* The value is not supposed to be used.  This is here to make it
diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 762c027..6b9e6b3 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -1305,27 +1305,27 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 	      if (TYPE_NFIELDS (type) != 0 || TYPE_NFN_FIELDS (type) != 0)
 		fprintf_filtered (stream, "\n");
 
-		for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); i++)
-		  {
-		    struct type *target = TYPE_TYPEDEF_FIELD_TYPE (type, i);
-
-		    /* Dereference the typedef declaration itself.  */
-		    gdb_assert (TYPE_CODE (target) == TYPE_CODE_TYPEDEF);
-		    target = TYPE_TARGET_TYPE (target);
-
-		    print_spaces_filtered (level + 4, stream);
-		    fprintf_filtered (stream, "typedef ");
-
-		    /* We want to print typedefs with substitutions
-		       from the template parameters or globally-known
-		       typedefs but not local typedefs.  */
-		    c_print_type (target,
-				  TYPE_TYPEDEF_FIELD_NAME (type, i),
-				  stream, show - 1, level + 4,
-				  &semi_local_flags);
-		    fprintf_filtered (stream, ";\n");
-		  }
-	      }
+	      for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); i++)
+		{
+		  struct type *target = TYPE_TYPEDEF_FIELD_TYPE (type, i);
+
+		  /* Dereference the typedef declaration itself.  */
+		  gdb_assert (TYPE_CODE (target) == TYPE_CODE_TYPEDEF);
+		  target = TYPE_TARGET_TYPE (target);
+
+		  print_spaces_filtered (level + 4, stream);
+		  fprintf_filtered (stream, "typedef ");
+
+		  /* We want to print typedefs with substitutions
+		     from the template parameters or globally-known
+		     typedefs but not local typedefs.  */
+		  c_print_type (target,
+				TYPE_TYPEDEF_FIELD_NAME (type, i),
+				stream, show - 1, level + 4,
+				&semi_local_flags);
+		  fprintf_filtered (stream, ";\n");
+		}
+	    }
 
 	    fprintfi_filtered (level, stream, "}");
 	  }
diff --git a/gdb/inflow.c b/gdb/inflow.c
index bbd2b12..4c80dbd 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -412,7 +412,7 @@ child_terminal_ours_1 (int output_only)
 
   if (tinfo->run_terminal != NULL || gdb_has_a_terminal () == 0)
     return;
-
+  else
     {
 #ifdef SIGTTOU
       /* Ignore this signal since it will happen when we try to set the
diff --git a/gdb/linux-record.c b/gdb/linux-record.c
index b9e49f3..29e299d 100644
--- a/gdb/linux-record.c
+++ b/gdb/linux-record.c
@@ -112,7 +112,7 @@ record_linux_sockaddr (struct regcache *regcache,
                             "memory at addr = 0x%s len = %d.\n",
                             phex_nz (len, tdep->size_pointer),
                             tdep->size_int);
-        return -1;
+      return -1;
     }
   addrlen = (int) extract_unsigned_integer (a, tdep->size_int, byte_order);
   if (addrlen <= 0 || addrlen > tdep->size_sockaddr)
@@ -150,7 +150,7 @@ record_linux_msghdr (struct regcache *regcache,
                             "len = %d.\n",
                             phex_nz (addr, tdep->size_pointer),
                             tdep->size_msghdr);
-        return -1;
+      return -1;
     }
 
   /* msg_name msg_namelen */
@@ -188,7 +188,7 @@ record_linux_msghdr (struct regcache *regcache,
                                     "len = %d.\n",
                                     phex_nz (addr,tdep->size_pointer),
                                     tdep->size_iovec);
-                return -1;
+              return -1;
             }
           tmpaddr = (CORE_ADDR) extract_unsigned_integer (iov,
                                                           tdep->size_pointer,
@@ -983,7 +983,7 @@ Do you want to stop the program?"),
                                         "memory at addr = 0x%s len = %d.\n",
                                         OUTPUT_REG (tmpulongest, tdep->arg2),
                                         tdep->size_ulong);
-                    return -1;
+                  return -1;
                 }
               tmpulongest = extract_unsigned_integer (a, tdep->size_ulong,
                                                       byte_order);
-- 
2.5.0