Fix -Wmisleading-indentation warning in top.c

Message ID 20191216235818.10307-1-simon.marchi@efficios.com
State New, archived
Headers

Commit Message

Simon Marchi Dec. 16, 2019, 11:58 p.m. UTC
  When building top.c with this clang (daily build from apt.llvm.org):

    $ clang++-10 --version
    clang version 10.0.0-+20191211091425+f99297176cd-1~exp1~20191211082036.1372

I get:

    /home/smarchi/src/binutils-gdb/gdb/top.c:1549:5: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation]
        fprintf_filtered (stream, _("\n\
        ^
    /home/smarchi/src/binutils-gdb/gdb/top.c:1543:3: note: previous statement is here
      if (SYSTEM_GDBINIT_DIR[0])
      ^

This looks like a legitimate warning, the fprintf_filtered is too much
indented.  Fix it, and at the same time add a bit of whitespace to make
this function easier to read.

gdb/ChangeLog:

	* top.c (print_gdb_configuration): Adjust indentation.
---
 gdb/top.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)
  

Comments

Pedro Alves Dec. 18, 2019, 4:42 p.m. UTC | #1
On 12/16/19 11:58 PM, Simon Marchi wrote:

> This looks like a legitimate warning, the fprintf_filtered is too much
> indented.  Fix it, and at the same time add a bit of whitespace to make
> this function easier to read.

Thanks for the extra whitespace.  I agree it helps.

> 
> gdb/ChangeLog:
> 
> 	* top.c (print_gdb_configuration): Adjust indentation.

OK.

> +  /* We assume "relocatable" will be printed at least once, thus we always
> +     print this text.  It's a reasonably safe assumption for now.  */
> +  fprintf_filtered (stream, _("\n\
>  (\"Relocatable\" means the directory can be moved with the GDB installation\n\
>  tree, and GDB will still find it.)\n\
>  "));
> 

Sounds like GCC should catch this too.

Thanks,
Pedro Alves
  
Pedro Alves Dec. 18, 2019, 5:10 p.m. UTC | #2
On 12/18/19 4:42 PM, Pedro Alves wrote:

> Sounds like GCC should catch this too.

It looks like it's the continuation line that prevents it.  Here's
a minimal reproducer:

void print (const char *);

void function1 (void)
{
  if (1)
    print ("\n");
    print ("\n"); // warns
}

void function2 (void)
{
  if (1)
    print ("hello\
\n");
    print ("\n"); // does not warn
}

I'll file a bug.

Thanks,
Pedro Alves
  
Simon Marchi Dec. 18, 2019, 5:28 p.m. UTC | #3
On 2019-12-18 12:10 p.m., Pedro Alves wrote:
> On 12/18/19 4:42 PM, Pedro Alves wrote:
> 
>> Sounds like GCC should catch this too.
> 
> It looks like it's the continuation line that prevents it.  Here's
> a minimal reproducer:
> 
> void print (const char *);
> 
> void function1 (void)
> {
>   if (1)
>     print ("\n");
>     print ("\n"); // warns
> }
> 
> void function2 (void)
> {
>   if (1)
>     print ("hello\
> \n");
>     print ("\n"); // does not warn
> }
> 
> I'll file a bug.
> 
> Thanks,
> Pedro Alves
> 

Oh, thanks a lot for doing this, I should have done it in the first place...

I pushed the patch.

Simon
  
Pedro Alves Dec. 18, 2019, 6:41 p.m. UTC | #4
On 12/18/19 5:10 PM, Pedro Alves wrote:

> I'll file a bug.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92995

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/top.c b/gdb/top.c
index bc300e47542a..6f366ffe69df 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1425,10 +1425,12 @@  print_gdb_configuration (struct ui_file *stream)
 This GDB was configured as follows:\n\
    configure --host=%s --target=%s\n\
 "), host_name, target_name);
+
   fprintf_filtered (stream, _("\
              --with-auto-load-dir=%s\n\
              --with-auto-load-safe-path=%s\n\
 "), AUTO_LOAD_DIR, AUTO_LOAD_SAFE_PATH);
+
 #if HAVE_LIBEXPAT
   fprintf_filtered (stream, _("\
              --with-expat\n\
@@ -1438,19 +1440,23 @@  This GDB was configured as follows:\n\
              --without-expat\n\
 "));
 #endif
+
   if (GDB_DATADIR[0])
     fprintf_filtered (stream, _("\
              --with-gdb-datadir=%s%s\n\
 "), GDB_DATADIR, GDB_DATADIR_RELOCATABLE ? " (relocatable)" : "");
+
 #ifdef ICONV_BIN
   fprintf_filtered (stream, _("\
              --with-iconv-bin=%s%s\n\
 "), ICONV_BIN, ICONV_BIN_RELOCATABLE ? " (relocatable)" : "");
 #endif
+
   if (JIT_READER_DIR[0])
     fprintf_filtered (stream, _("\
              --with-jit-reader-dir=%s%s\n\
 "), JIT_READER_DIR, JIT_READER_DIR_RELOCATABLE ? " (relocatable)" : "");
+
 #if HAVE_LIBUNWIND_IA64_H
   fprintf_filtered (stream, _("\
              --with-libunwind-ia64\n\
@@ -1460,6 +1466,7 @@  This GDB was configured as follows:\n\
              --without-libunwind-ia64\n\
 "));
 #endif
+
 #if HAVE_LIBLZMA
   fprintf_filtered (stream, _("\
              --with-lzma\n\
@@ -1469,6 +1476,7 @@  This GDB was configured as follows:\n\
              --without-lzma\n\
 "));
 #endif
+
 #if HAVE_LIBBABELTRACE
     fprintf_filtered (stream, _("\
              --with-babeltrace\n\
@@ -1478,6 +1486,7 @@  This GDB was configured as follows:\n\
              --without-babeltrace\n\
 "));
 #endif
+
 #if HAVE_LIBIPT
     fprintf_filtered (stream, _("\
              --with-intel-pt\n\
@@ -1487,6 +1496,7 @@  This GDB was configured as follows:\n\
              --without-intel-pt\n\
 "));
 #endif
+
 #if HAVE_LIBMPFR
     fprintf_filtered (stream, _("\
              --with-mpfr\n\
@@ -1496,6 +1506,7 @@  This GDB was configured as follows:\n\
              --without-mpfr\n\
 "));
 #endif
+
 #ifdef WITH_PYTHON_PATH
   fprintf_filtered (stream, _("\
              --with-python=%s%s\n\
@@ -1505,6 +1516,7 @@  This GDB was configured as follows:\n\
              --without-python\n\
 "));
 #endif
+
 #if HAVE_GUILE
   fprintf_filtered (stream, _("\
              --with-guile\n\
@@ -1514,6 +1526,7 @@  This GDB was configured as follows:\n\
              --without-guile\n\
 "));
 #endif
+
 #if HAVE_SOURCE_HIGHLIGHT
   fprintf_filtered (stream, _("\
              --enable-source-highlight\n\
@@ -1523,30 +1536,36 @@  This GDB was configured as follows:\n\
              --disable-source-highlight\n\
 "));
 #endif
+
 #ifdef RELOC_SRCDIR
   fprintf_filtered (stream, _("\
              --with-relocated-sources=%s\n\
 "), RELOC_SRCDIR);
 #endif
+
   if (DEBUGDIR[0])
     fprintf_filtered (stream, _("\
              --with-separate-debug-dir=%s%s\n\
 "), DEBUGDIR, DEBUGDIR_RELOCATABLE ? " (relocatable)" : "");
+
   if (TARGET_SYSTEM_ROOT[0])
     fprintf_filtered (stream, _("\
              --with-sysroot=%s%s\n\
 "), TARGET_SYSTEM_ROOT, TARGET_SYSTEM_ROOT_RELOCATABLE ? " (relocatable)" : "");
+
   if (SYSTEM_GDBINIT[0])
     fprintf_filtered (stream, _("\
              --with-system-gdbinit=%s%s\n\
 "), SYSTEM_GDBINIT, SYSTEM_GDBINIT_RELOCATABLE ? " (relocatable)" : "");
+
   if (SYSTEM_GDBINIT_DIR[0])
     fprintf_filtered (stream, _("\
              --with-system-gdbinit-dir=%s%s\n\
 "), SYSTEM_GDBINIT_DIR, SYSTEM_GDBINIT_DIR_RELOCATABLE ? " (relocatable)" : "");
-    /* We assume "relocatable" will be printed at least once, thus we always
-       print this text.  It's a reasonably safe assumption for now.  */
-    fprintf_filtered (stream, _("\n\
+
+  /* We assume "relocatable" will be printed at least once, thus we always
+     print this text.  It's a reasonably safe assumption for now.  */
+  fprintf_filtered (stream, _("\n\
 (\"Relocatable\" means the directory can be moved with the GDB installation\n\
 tree, and GDB will still find it.)\n\
 "));