[RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

classic Classic list List threaded Threaded
50 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

Tom Tromey-2
I happened to notice an unused variable in the newly minted Rust code,
so I decided to take a look at adding some warnings to prevent this.

This series adds -Wunused-but-set-parameter and
-Wunused-but-set-variable to the build.  First come the patches to
remove all the warnings, and the final patch enables the compiler
flags.

I've split up the patches for easier review, and according to the type
of patch.  I think they're generally self-explanatory.

I built and regression tested this using --enable-targets=all on
x86-64 Fedora 23.  However, I could not update the various nat-*
files, so there are probably unfixed warnings lurking there.

Tom

Reply | Threaded
Open this post in threaded view
|

[RFA 5/6] Remove unused variables

Tom Tromey-2
This patch removes set-but-unused variables.  This holds all the
removals I consider to be simple and relatively uncontroversial.

2016-06-06  Tom Tromey  <[hidden email]>

        * mips-tdep.c (micromips_scan_prologue): Remove "frame_addr".
        (mips_o32_push_dummy_call): Remove "stack_used_p".
        * aarch64-tdep.c (aarch64_record_data_proc_imm): Remove
        "insn_bit28".
        * rust-lang.c (rust_print_type): Remove "len".
        * rust-exp.y (super_name): Remove "current_len".
        * python/py-framefilter.c (py_print_type): Remove "type".
        * mdebugread.c (parse_partial_symbols): Remove
        "past_first_source_file".
        <N_SO>: Remove "valu", "first_so_symnum", "prev_textlow_not_set".
        * m2-valprint.c (m2_print_unbounded_array): Remove
        "content_type".
        (m2_val_print): Remove "i".
        * linespec.c (unexpected_linespec_error): Remove "cleanup".
        * f-valprint.c (f_val_print): Remove "i".
        * elfread.c (elf_symtab_read): Remove "offset".
        * dwarf2-frame.c (dwarf2_fetch_cfa_info): Remove "addr_size".
---
 gdb/ChangeLog               | 20 ++++++++++++++++++++
 gdb/aarch64-tdep.c          |  3 +--
 gdb/dwarf2-frame.c          |  2 --
 gdb/elfread.c               |  3 ---
 gdb/f-valprint.c            |  5 ++---
 gdb/linespec.c              |  3 +--
 gdb/m2-valprint.c           |  4 ----
 gdb/mdebugread.c            | 18 +-----------------
 gdb/mips-tdep.c             | 13 +------------
 gdb/python/py-framefilter.c |  2 --
 gdb/rust-exp.y              |  4 +---
 gdb/rust-lang.c             |  3 +--
 12 files changed, 28 insertions(+), 52 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b164911..eec280d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,25 @@
 2016-06-06  Tom Tromey  <[hidden email]>
 
+ * mips-tdep.c (micromips_scan_prologue): Remove "frame_addr".
+ (mips_o32_push_dummy_call): Remove "stack_used_p".
+ * aarch64-tdep.c (aarch64_record_data_proc_imm): Remove
+ "insn_bit28".
+ * rust-lang.c (rust_print_type): Remove "len".
+ * rust-exp.y (super_name): Remove "current_len".
+ * python/py-framefilter.c (py_print_type): Remove "type".
+ * mdebugread.c (parse_partial_symbols): Remove
+ "past_first_source_file".
+ <N_SO>: Remove "valu", "first_so_symnum", "prev_textlow_not_set".
+ * m2-valprint.c (m2_print_unbounded_array): Remove
+ "content_type".
+ (m2_val_print): Remove "i".
+ * linespec.c (unexpected_linespec_error): Remove "cleanup".
+ * f-valprint.c (f_val_print): Remove "i".
+ * elfread.c (elf_symtab_read): Remove "offset".
+ * dwarf2-frame.c (dwarf2_fetch_cfa_info): Remove "addr_size".
+
+2016-06-06  Tom Tromey  <[hidden email]>
+
  * arch-utils.c (default_skip_permanent_breakpoint): Remove
  "bp_insn".
  * disasm.c (do_assembly_only): Remove "num_displayed".
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 88fcf4b..92b57e5 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2981,11 +2981,10 @@ aarch64_record_data_proc_reg (insn_decode_record *aarch64_insn_r)
 static unsigned int
 aarch64_record_data_proc_imm (insn_decode_record *aarch64_insn_r)
 {
-  uint8_t reg_rd, insn_bit28, insn_bit23, insn_bits24_27, setflags;
+  uint8_t reg_rd, insn_bit23, insn_bits24_27, setflags;
   uint32_t record_buf[4];
 
   reg_rd = bits (aarch64_insn_r->aarch64_insn, 0, 4);
-  insn_bit28 = bit (aarch64_insn_r->aarch64_insn, 28);
   insn_bit23 = bit (aarch64_insn_r->aarch64_insn, 23);
   insn_bits24_27 = bits (aarch64_insn_r->aarch64_insn, 24, 27);
 
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 2f6355a..11258ea 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -908,7 +908,6 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
   struct dwarf2_fde *fde;
   CORE_ADDR text_offset;
   struct dwarf2_frame_state fs;
-  int addr_size;
 
   memset (&fs, 0, sizeof (struct dwarf2_frame_state));
 
@@ -923,7 +922,6 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
   fs.data_align = fde->cie->data_alignment_factor;
   fs.code_align = fde->cie->code_alignment_factor;
   fs.retaddr_column = fde->cie->return_address_register;
-  addr_size = fde->cie->addr_size;
 
   /* Check for "quirks" - known bugs in producers.  */
   dwarf2_frame_find_quirks (&fs, fde);
diff --git a/gdb/elfread.c b/gdb/elfread.c
index d4400bb..e90466b 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -233,7 +233,6 @@ elf_symtab_read (struct objfile *objfile, int type,
   asymbol *sym;
   long i;
   CORE_ADDR symaddr;
-  CORE_ADDR offset;
   enum minimal_symbol_type ms_type;
   /* Name of the last file symbol.  This is either a constant string or is
      saved on the objfile's filename cache.  */
@@ -263,8 +262,6 @@ elf_symtab_read (struct objfile *objfile, int type,
   continue;
  }
 
-      offset = ANOFFSET (objfile->section_offsets,
- gdb_bfd_section_index (objfile->obfd, sym->section));
       if (type == ST_DYNAMIC
   && sym->section == bfd_und_section_ptr
   && (sym->flags & BSF_FUNCTION))
diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c
index 1264737..f31b4c7 100644
--- a/gdb/f-valprint.c
+++ b/gdb/f-valprint.c
@@ -218,7 +218,6 @@ f_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 {
   struct gdbarch *gdbarch = get_type_arch (type);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  unsigned int i = 0; /* Number of characters printed.  */
   struct type *elttype;
   CORE_ADDR addr;
   int index;
@@ -292,8 +291,8 @@ f_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
     {
       if (want_space)
  fputs_filtered (" ", stream);
-      i = val_print_string (TYPE_TARGET_TYPE (type), NULL, addr, -1,
-    stream, options);
+      val_print_string (TYPE_TARGET_TYPE (type), NULL, addr, -1,
+ stream, options);
     }
   return;
  }
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 7162163..ccedec8 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1519,10 +1519,9 @@ unexpected_linespec_error (linespec_parser *parser)
       || token.type == LSTOKEN_KEYWORD)
     {
       char *string;
-      struct cleanup *cleanup;
 
       string = copy_token_string (token);
-      cleanup = make_cleanup (xfree, string);
+      make_cleanup (xfree, string);
       throw_error (GENERIC_ERROR,
    _("malformed linespec error: unexpected %s, \"%s\""),
    token_type_strings[token.type], string);
diff --git a/gdb/m2-valprint.c b/gdb/m2-valprint.c
index 2d3a32f..a53aa84 100644
--- a/gdb/m2-valprint.c
+++ b/gdb/m2-valprint.c
@@ -162,13 +162,11 @@ m2_print_unbounded_array (struct type *type, const gdb_byte *valaddr,
   struct ui_file *stream, int recurse,
   const struct value_print_options *options)
 {
-  struct type *content_type;
   CORE_ADDR addr;
   LONGEST len;
   struct value *val;
 
   type = check_typedef (type);
-  content_type = TYPE_TARGET_TYPE (TYPE_FIELD_TYPE (type, 0));
 
   addr = unpack_pointer (TYPE_FIELD_TYPE (type, 0),
  (TYPE_FIELD_BITPOS (type, 0) / 8) +
@@ -316,7 +314,6 @@ m2_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
       const struct value_print_options *options)
 {
   struct gdbarch *gdbarch = get_type_arch (type);
-  unsigned int i = 0; /* Number of characters printed.  */
   unsigned len;
   struct type *elttype;
   CORE_ADDR addr;
@@ -355,7 +352,6 @@ m2_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
       LA_PRINT_STRING (stream, TYPE_TARGET_TYPE (type),
        valaddr + embedded_offset, len, NULL,
        0, options);
-      i = len;
     }
   else
     {
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index c46e880..a6a2efe 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -2368,7 +2368,6 @@ parse_partial_symbols (struct objfile *objfile)
   SYMR sh;
   struct partial_symtab *pst;
   int textlow_not_set = 1;
-  int past_first_source_file = 0;
 
   /* List of current psymtab's include files.  */
   const char **psymtab_include_list;
@@ -2957,16 +2956,8 @@ parse_partial_symbols (struct objfile *objfile)
 
   case N_SO:
     {
-      CORE_ADDR valu;
       static int prev_so_symnum = -10;
-      static int first_so_symnum;
       const char *p;
-      int prev_textlow_not_set;
-
-      valu = sh.value + ANOFFSET (objfile->section_offsets,
-  SECT_OFF_TEXT (objfile));
-
-      prev_textlow_not_set = textlow_not_set;
 
       /* A zero value is probably an indication for the
  SunPRO 3.0 compiler.  dbx_end_psymtab explicitly tests
@@ -2974,19 +2965,12 @@ parse_partial_symbols (struct objfile *objfile)
 
       if (sh.value == 0
   && gdbarch_sofun_address_maybe_missing (gdbarch))
- {
-  textlow_not_set = 1;
-  valu = 0;
- }
+ textlow_not_set = 1;
       else
  textlow_not_set = 0;
 
-      past_first_source_file = 1;
-
       if (prev_so_symnum != symnum - 1)
  { /* Here if prev stab wasn't N_SO.  */
-  first_so_symnum = symnum;
-
   if (pst)
     {
       pst = (struct partial_symtab *) 0;
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 6098f71..4e4d79e 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -2939,7 +2939,6 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
   int non_prologue_insns = 0;
   long frame_offset = 0; /* Size of stack frame.  */
   long frame_adjust = 0; /* Offset of FP from SP.  */
-  CORE_ADDR frame_addr = 0; /* Value of $30, used as frame pointer.  */
   int prev_delay_slot = 0;
   int in_delay_slot;
   CORE_ADDR prev_pc;
@@ -3068,7 +3067,6 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
       else if (sreg == MIPS_SP_REGNUM && dreg == 30)
  /* (D)ADDIU $fp, $sp, imm */
  {
-  frame_addr = sp + offset;
   frame_adjust = offset;
   frame_reg = 30;
  }
@@ -3144,10 +3142,7 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
       dreg = b5s5_reg (insn);
       if (sreg == MIPS_SP_REGNUM && dreg == 30)
  /* MOVE  $fp, $sp */
- {
-  frame_addr = sp;
-  frame_reg = 30;
- }
+ frame_reg = 30;
       else if ((sreg & 0x1c) != 0x4)
  /* MOVE  reg, $a0-$a3 */
  this_non_prologue_insn = 1;
@@ -5502,8 +5497,6 @@ mips_o32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
     }
   while (len > 0)
     {
-      /* Remember if the argument was written to the stack.  */
-      int stack_used_p = 0;
       int partial_len = (len < MIPS32_REGSIZE ? len : MIPS32_REGSIZE);
 
       if (mips_debug)
@@ -5518,7 +5511,6 @@ mips_o32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
      promoted to int before being stored?  */
   int longword_offset = 0;
   CORE_ADDR addr;
-  stack_used_p = 1;
 
   if (mips_debug)
     {
@@ -5960,8 +5952,6 @@ mips_o64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   && len % MIPS64_REGSIZE != 0);
   while (len > 0)
     {
-      /* Remember if the argument was written to the stack.  */
-      int stack_used_p = 0;
       int partial_len = (len < MIPS64_REGSIZE ? len : MIPS64_REGSIZE);
 
       if (mips_debug)
@@ -5976,7 +5966,6 @@ mips_o64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
      promoted to int before being stored?  */
   int longword_offset = 0;
   CORE_ADDR addr;
-  stack_used_p = 1;
   if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
     {
       if ((typecode == TYPE_CODE_INT
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index aa25911..b663f50 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -211,13 +211,11 @@ py_print_type (struct ui_out *out, struct value *val)
 
   TRY
     {
-      struct type *type;
       struct ui_file *stb;
       struct cleanup *cleanup;
 
       stb = mem_fileopen ();
       cleanup = make_cleanup_ui_file_delete (stb);
-      type = check_typedef (value_type (val));
       type_print (value_type (val), "", stb, -1);
       ui_out_field_stream (out, "type", stb);
       do_cleanups (cleanup);
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index c1a863c..05e59db 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -968,17 +968,15 @@ super_name (const struct rust_op *ident, unsigned int n_supers)
       int i;
       int len;
       VEC (int) *offsets = NULL;
-      unsigned int current_len, previous_len;
+      unsigned int current_len;
       struct cleanup *cleanup;
 
       cleanup = make_cleanup (VEC_cleanup (int), &offsets);
       current_len = cp_find_first_component (scope);
-      previous_len = 0;
       while (scope[current_len] != '\0')
  {
   VEC_safe_push (int, offsets, current_len);
   gdb_assert (scope[current_len] == ':');
-  previous_len = current_len;
   /* The "::".  */
   current_len += 2;
   current_len += cp_find_first_component (scope
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 5df99ce..a7b4aae 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -840,14 +840,13 @@ rust_print_type (struct type *type, const char *varstring,
     case TYPE_CODE_UNION:
       {
  /* ADT enums */
- int i, len = 0;
+ int i;
 
  fputs_filtered ("enum ", stream);
  if (TYPE_TAG_NAME (type) != NULL)
   {
     fputs_filtered (TYPE_TAG_NAME (type), stream);
     fputs_filtered (" ", stream);
-    len = strlen (TYPE_TAG_NAME (type));
   }
  fputs_filtered ("{\n", stream);      
 
--
2.5.5

Reply | Threaded
Open this post in threaded view
|

[RFA 6/6] Add -Wunused-but-set-* to build

Tom Tromey-2
In reply to this post by Tom Tromey-2
This adds -Wunused-but-set-variable and -Wunused-but-set-parameter to
configure.

2016-06-06  Tom Tromey  <[hidden email]>

        * configure: Rebuild.
        * warning.m4 (AM_GDB_WARNINGS) <build_warnings>: Add
        -Wunused-but-set-parameter, -Wunused-but-set-variable.

2016-06-06  Tom Tromey  <[hidden email]>

        * configure: Rebuild.
---
 gdb/ChangeLog           | 6 ++++++
 gdb/configure           | 2 +-
 gdb/gdbserver/ChangeLog | 4 ++++
 gdb/gdbserver/configure | 2 +-
 gdb/warning.m4          | 2 +-
 5 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index eec280d..9c0ff17 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2016-06-06  Tom Tromey  <[hidden email]>
 
+ * configure: Rebuild.
+ * warning.m4 (AM_GDB_WARNINGS) <build_warnings>: Add
+ -Wunused-but-set-parameter, -Wunused-but-set-variable.
+
+2016-06-06  Tom Tromey  <[hidden email]>
+
  * mips-tdep.c (micromips_scan_prologue): Remove "frame_addr".
  (mips_o32_push_dummy_call): Remove "stack_used_p".
  * aarch64-tdep.c (aarch64_record_data_proc_imm): Remove
diff --git a/gdb/configure b/gdb/configure
index 60ea884..957d0e4 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -14242,7 +14242,7 @@ fi
 build_warnings="-Wall -Wpointer-arith \
 -Wno-unused -Wunused-value -Wunused-function \
 -Wno-switch -Wno-char-subscripts \
--Wempty-body"
+-Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable"
 
 # Now add in C and C++ specific options, depending on mode.
 if test "$enable_build_with_cxx" = "yes"; then
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 079507a..8bef0bd 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,7 @@
+2016-06-06  Tom Tromey  <[hidden email]>
+
+ * configure: Rebuild.
+
 2016-06-02  Jon Turney  <[hidden email]>
 
  * win32-low.c (win32_create_inferior): Add pointer casts for C++.
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 746218e..2926deb 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -6291,7 +6291,7 @@ fi
 build_warnings="-Wall -Wpointer-arith \
 -Wno-unused -Wunused-value -Wunused-function \
 -Wno-switch -Wno-char-subscripts \
--Wempty-body"
+-Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable"
 
 # Now add in C and C++ specific options, depending on mode.
 if test "$enable_build_with_cxx" = "yes"; then
diff --git a/gdb/warning.m4 b/gdb/warning.m4
index 55f1eb3..8d7ce68 100644
--- a/gdb/warning.m4
+++ b/gdb/warning.m4
@@ -39,7 +39,7 @@ fi
 build_warnings="-Wall -Wpointer-arith \
 -Wno-unused -Wunused-value -Wunused-function \
 -Wno-switch -Wno-char-subscripts \
--Wempty-body"
+-Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable"
 
 # Now add in C and C++ specific options, depending on mode.
 if test "$enable_build_with_cxx" = "yes"; then
--
2.5.5

Reply | Threaded
Open this post in threaded view
|

[RFA 1/6] Change reopen_exec_file to check result of stat

Tom Tromey-2
In reply to this post by Tom Tromey-2
This seems to be a real bug found by -Wunused-but-set-variable.  If
"stat" fails for some reason, gdb would use the uninitialized "st".

2016-06-06  Tom Tromey  <[hidden email]>

        * corefile.c (reopen_exec_file): Only examine st.st_mtime if stat
        succeeded.
---
 gdb/ChangeLog  | 5 +++++
 gdb/corefile.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index af4ddcc..649bb0b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2016-06-06  Tom Tromey  <[hidden email]>
+
+ * corefile.c (reopen_exec_file): Only examine st.st_mtime if stat
+ succeeded.
+
 2016-06-02  Jon Turney  <[hidden email]>
 
  * windows-nat.c (handle_output_debug_string): Return type of
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 6cc2afc..64de931 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -144,7 +144,7 @@ reopen_exec_file (void)
   cleanups = make_cleanup (xfree, filename);
   res = stat (filename, &st);
 
-  if (exec_bfd_mtime && exec_bfd_mtime != st.st_mtime)
+  if (res == 0 && exec_bfd_mtime && exec_bfd_mtime != st.st_mtime)
     exec_file_attach (filename, 0);
   else
     /* If we accessed the file since last opening it, close it now;
--
2.5.5

Reply | Threaded
Open this post in threaded view
|

[RFA 3/6] Comment out some unused overlay code

Tom Tromey-2
In reply to this post by Tom Tromey-2
This patch comments out some unneeded initializations in overlay code
in symfile.c.  Normally I would not comment out code like this, but
the uses of "size" in these functions are commented out.  If you'd
prefer I could just delete the comments.

2016-06-06  Tom Tromey  <[hidden email]>

        * symfile.c (simple_overlay_update_1): Comment out initialization
        of "size".
        (simple_overlay_update): Likewise.
---
 gdb/ChangeLog | 6 ++++++
 gdb/symfile.c | 8 ++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fbd7051..9839c55 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2016-06-06  Tom Tromey  <[hidden email]>
 
+ * symfile.c (simple_overlay_update_1): Comment out initialization
+ of "size".
+ (simple_overlay_update): Likewise.
+
+2016-06-06  Tom Tromey  <[hidden email]>
+
  * tui/tui-winsource.c (tui_show_source_line): Use
  ATTRIBUTE_UNUSED.
  * tui/tui-io.c (tui_puts): Use ATTRIBUTE_UNUSED.
diff --git a/gdb/symfile.c b/gdb/symfile.c
index b244332..5de2cbf 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3630,14 +3630,14 @@ simple_read_overlay_table (void)
 static int
 simple_overlay_update_1 (struct obj_section *osect)
 {
-  int i, size;
+  int i;
   bfd *obfd = osect->objfile->obfd;
   asection *bsect = osect->the_bfd_section;
   struct gdbarch *gdbarch = get_objfile_arch (osect->objfile);
   int word_size = gdbarch_long_bit (gdbarch) / TARGET_CHAR_BIT;
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
-  size = bfd_get_section_size (osect->the_bfd_section);
+  /* size = bfd_get_section_size (osect->the_bfd_section); */
   for (i = 0; i < cache_novlys; i++)
     if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
  && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
@@ -3706,11 +3706,11 @@ simple_overlay_update (struct obj_section *osect)
   ALL_OBJSECTIONS (objfile, osect)
     if (section_is_overlay (osect))
     {
-      int i, size;
+      int i;
       bfd *obfd = osect->objfile->obfd;
       asection *bsect = osect->the_bfd_section;
 
-      size = bfd_get_section_size (bsect);
+      /* size = bfd_get_section_size (bsect); */
       for (i = 0; i < cache_novlys; i++)
  if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
     && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
--
2.5.5

Reply | Threaded
Open this post in threaded view
|

[RFA 4/6] Remove some variables but call functions for side effects

Tom Tromey-2
In reply to this post by Tom Tromey-2
This patch consolidates the (possibly-questionable) spots where we
remove a declaration but continue to call some function for side
effects.  In a couple of cases it wasn't entirely clear to me that
this mattered; and in some other cases it might be more aesthetically
pleasing to use ATTRIBUTE_UNUSED.  So, I broke this out into a
separate patch for simpler review.

2016-06-06  Tom Tromey  <[hidden email]>

        * arch-utils.c (default_skip_permanent_breakpoint): Remove
        "bp_insn".
        * disasm.c (do_assembly_only): Remove "num_displayed".
        * dwarf2read.c (read_abbrev_offset): Remove "length".
        (dwarf_decode_macro_bytes) <DW_MACINFO_vendor_ext>: Remove
        "constant".
        * m32c-tdep.c (make_regs): Remove "r2hl", "r3hl", and "intbhl".
        * microblaze-tdep.c (microblaze_frame_cache): Remove "func".
        * tracefile.c (trace_save): Remove "status".
---
 gdb/ChangeLog         | 12 ++++++++++++
 gdb/arch-utils.c      |  3 +--
 gdb/disasm.c          |  4 +---
 gdb/dwarf2read.c      |  9 +++++----
 gdb/m32c-tdep.c       |  9 +++------
 gdb/microblaze-tdep.c |  4 ++--
 gdb/tracefile.c       |  7 ++++---
 7 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9839c55..b164911 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,17 @@
 2016-06-06  Tom Tromey  <[hidden email]>
 
+ * arch-utils.c (default_skip_permanent_breakpoint): Remove
+ "bp_insn".
+ * disasm.c (do_assembly_only): Remove "num_displayed".
+ * dwarf2read.c (read_abbrev_offset): Remove "length".
+ (dwarf_decode_macro_bytes) <DW_MACINFO_vendor_ext>: Remove
+ "constant".
+ * m32c-tdep.c (make_regs): Remove "r2hl", "r3hl", and "intbhl".
+ * microblaze-tdep.c (microblaze_frame_cache): Remove "func".
+ * tracefile.c (trace_save): Remove "status".
+
+2016-06-06  Tom Tromey  <[hidden email]>
+
  * symfile.c (simple_overlay_update_1): Comment out initialization
  of "size".
  (simple_overlay_update): Likewise.
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 604042f..53121bc 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -857,10 +857,9 @@ default_skip_permanent_breakpoint (struct regcache *regcache)
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   CORE_ADDR current_pc = regcache_read_pc (regcache);
-  const gdb_byte *bp_insn;
   int bp_len;
 
-  bp_insn = gdbarch_breakpoint_from_pc (gdbarch, &current_pc, &bp_len);
+  gdbarch_breakpoint_from_pc (gdbarch, &current_pc, &bp_len);
   current_pc += bp_len;
   regcache_write_pc (regcache, current_pc);
 }
diff --git a/gdb/disasm.c b/gdb/disasm.c
index f611721..bd1f760 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -737,13 +737,11 @@ do_assembly_only (struct gdbarch *gdbarch, struct ui_out *uiout,
   CORE_ADDR low, CORE_ADDR high,
   int how_many, int flags, struct ui_file *stb)
 {
-  int num_displayed = 0;
   struct cleanup *ui_out_chain;
 
   ui_out_chain = make_cleanup_ui_out_list_begin_end (uiout, "asm_insns");
 
-  num_displayed = dump_insns (gdbarch, uiout, di, low, high, how_many,
-                              flags, stb, NULL);
+  dump_insns (gdbarch, uiout, di, low, high, how_many, flags, stb, NULL);
 
   do_cleanups (ui_out_chain);
 }
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 6658a38..58b502b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -4497,12 +4497,12 @@ read_abbrev_offset (struct dwarf2_section_info *section,
 {
   bfd *abfd = get_section_bfd_owner (section);
   const gdb_byte *info_ptr;
-  unsigned int length, initial_length_size, offset_size;
+  unsigned int initial_length_size, offset_size;
   sect_offset abbrev_offset;
 
   dwarf2_read_section (dwarf2_per_objfile->objfile, section);
   info_ptr = section->buffer + offset.sect_off;
-  length = read_initial_length (abfd, info_ptr, &initial_length_size);
+  read_initial_length (abfd, info_ptr, &initial_length_size);
   offset_size = initial_length_size == 4 ? 4 : 8;
   info_ptr += initial_length_size + 2 /*version*/;
   abbrev_offset.sect_off = read_offset_1 (abfd, info_ptr, offset_size);
@@ -21578,9 +21578,10 @@ dwarf_decode_macro_bytes (bfd *abfd,
   if (!section_is_gnu)
     {
       unsigned int bytes_read;
-      int constant;
 
-      constant = read_unsigned_leb128 (abfd, mac_ptr, &bytes_read);
+      /* This reads the constant, but since we don't recognize
+ any vendor extensions, we ignore it.  */
+      read_unsigned_leb128 (abfd, mac_ptr, &bytes_read);
       mac_ptr += bytes_read;
       read_direct_string (abfd, mac_ptr, &bytes_read);
       mac_ptr += bytes_read;
diff --git a/gdb/m32c-tdep.c b/gdb/m32c-tdep.c
index 59927a6..1e98f73 100644
--- a/gdb/m32c-tdep.c
+++ b/gdb/m32c-tdep.c
@@ -823,9 +823,6 @@ make_regs (struct gdbarch *arch)
   struct m32c_reg *sp;
   struct m32c_reg *r0hl;
   struct m32c_reg *r1hl;
-  struct m32c_reg *r2hl;
-  struct m32c_reg *r3hl;
-  struct m32c_reg *intbhl;
   struct m32c_reg *r2r0;
   struct m32c_reg *r3r1;
   struct m32c_reg *r3r1r2r0;
@@ -889,9 +886,9 @@ make_regs (struct gdbarch *arch)
 
   r0hl        = CHL (r0, tdep->int8);
   r1hl        = CHL (r1, tdep->int8);
-  r2hl        = CHL (r2, tdep->int8);
-  r3hl        = CHL (r3, tdep->int8);
-  intbhl      = CHL (intb, tdep->int16);
+  CHL (r2, tdep->int8);
+  CHL (r3, tdep->int8);
+  CHL (intb, tdep->int16);
 
   r2r0        = CCAT (r2,   r0,   tdep->int32);
   r3r1        = CCAT (r3,   r1,   tdep->int32);
diff --git a/gdb/microblaze-tdep.c b/gdb/microblaze-tdep.c
index 9cfe3fe..665ec0c 100644
--- a/gdb/microblaze-tdep.c
+++ b/gdb/microblaze-tdep.c
@@ -437,7 +437,6 @@ microblaze_frame_cache (struct frame_info *next_frame, void **this_cache)
 {
   struct microblaze_frame_cache *cache;
   struct gdbarch *gdbarch = get_frame_arch (next_frame);
-  CORE_ADDR func;
   int rn;
 
   if (*this_cache)
@@ -451,7 +450,8 @@ microblaze_frame_cache (struct frame_info *next_frame, void **this_cache)
   for (rn = 0; rn < gdbarch_num_regs (gdbarch); rn++)
     cache->register_offsets[rn] = -1;
 
-  func = get_frame_func (next_frame);
+  /* Call for side effects.  */
+  get_frame_func (next_frame);
 
   cache->pc = get_frame_address_in_block (next_frame);
 
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index c32bcf4..56fb0d2 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -57,7 +57,6 @@ trace_save (const char *filename, struct trace_file_writer *writer,
     int target_does_save)
 {
   struct trace_status *ts = current_trace_status ();
-  int status;
   struct uploaded_tp *uploaded_tps = NULL, *utp;
   struct uploaded_tsv *uploaded_tsvs = NULL, *utsv;
 
@@ -77,8 +76,10 @@ trace_save (const char *filename, struct trace_file_writer *writer,
     }
 
   /* Get the trace status first before opening the file, so if the
-     target is losing, we can get out without touching files.  */
-  status = target_get_trace_status (ts);
+     target is losing, we can get out without touching files.  Since
+     we're just calling this for side effects, we ignore the
+     result.  */
+  target_get_trace_status (ts);
 
   writer->ops->start (writer, filename);
 
--
2.5.5

Reply | Threaded
Open this post in threaded view
|

[RFA 2/6] Use ATTRIBUTE_UNUSED in some places

Tom Tromey-2
In reply to this post by Tom Tromey-2
A few spots needed ATTRIBUTE_UNUSED to cope with the new warnings.

In the TUI this happens due to the odd way that "getyx" works -- it
essentially uses references, so a variable must be passed in, even if
the code doesn't need to use it.

The case in inflow.c is just a mass of ifdefs; and while the only use
of "result" is guarded by "#if 0", I thought it simplest to leave it
all in place.

2016-06-06  Tom Tromey  <[hidden email]>

        * tui/tui-winsource.c (tui_show_source_line): Use
        ATTRIBUTE_UNUSED.
        * tui/tui-io.c (tui_puts): Use ATTRIBUTE_UNUSED.
        (tui_redisplay_readline): Likewise.
        * inflow.c (child_terminal_ours_1): Use ATTRIBUTE_UNUSED.
---
 gdb/ChangeLog           | 8 ++++++++
 gdb/inflow.c            | 2 +-
 gdb/tui/tui-io.c        | 6 ++++--
 gdb/tui/tui-winsource.c | 3 ++-
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 649bb0b..fbd7051 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2016-06-06  Tom Tromey  <[hidden email]>
 
+ * tui/tui-winsource.c (tui_show_source_line): Use
+ ATTRIBUTE_UNUSED.
+ * tui/tui-io.c (tui_puts): Use ATTRIBUTE_UNUSED.
+ (tui_redisplay_readline): Likewise.
+ * inflow.c (child_terminal_ours_1): Use ATTRIBUTE_UNUSED.
+
+2016-06-06  Tom Tromey  <[hidden email]>
+
  * corefile.c (reopen_exec_file): Only examine st.st_mtime if stat
  succeeded.
 
diff --git a/gdb/inflow.c b/gdb/inflow.c
index 4c80dbd..9b94b29 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -419,7 +419,7 @@ child_terminal_ours_1 (int output_only)
          pgrp.  */
       sighandler_t osigttou = NULL;
 #endif
-      int result;
+      int result ATTRIBUTE_UNUSED;
 
 #ifdef SIGTTOU
       if (job_control)
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 3fa32db..35025ce 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -172,7 +172,8 @@ tui_puts (const char *string)
   /* Expand TABs, since ncurses on MS-Windows doesn't.  */
   if (c == '\t')
     {
-      int line, col;
+      int line ATTRIBUTE_UNUSED;
+      int col;
 
       getyx (w, line, col);
       do
@@ -198,7 +199,8 @@ tui_redisplay_readline (void)
 {
   int prev_col;
   int height;
-  int col, line;
+  int col;
+  int line ATTRIBUTE_UNUSED;
   int c_pos;
   int c_line;
   int in;
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 48975b6..48bfbe4 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -273,7 +273,8 @@ static void
 tui_show_source_line (struct tui_win_info *win_info, int lineno)
 {
   struct tui_win_element *line;
-  int x, y;
+  int x;
+  int y ATTRIBUTE_UNUSED;
 
   line = win_info->generic.content[lineno - 1];
   if (line->which_element.source.is_exec_point)
--
2.5.5

Reply | Threaded
Open this post in threaded view
|

Re: [RFA 6/6] Add -Wunused-but-set-* to build

Trevor Saunders
In reply to this post by Tom Tromey-2
On Mon, Jun 06, 2016 at 03:33:32PM -0600, Tom Tromey wrote:
> This adds -Wunused-but-set-variable and -Wunused-but-set-parameter to
> configure.

 great, I started on that a little while ago but didn't get to pushing
 these bits upstream.

> +++ b/gdb/warning.m4
> @@ -39,7 +39,7 @@ fi
>  build_warnings="-Wall -Wpointer-arith \
>  -Wno-unused -Wunused-value -Wunused-function \
>  -Wno-switch -Wno-char-subscripts \
> --Wempty-body"
> +-Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable"

isn't everything in -Wunused enabled now? can we just delete -Wno-unused
and use -Wall to get us -Wunused?

Trev

Reply | Threaded
Open this post in threaded view
|

Re: [RFA 6/6] Add -Wunused-but-set-* to build

Tom Tromey-2
>>>>> "Trevor" == Trevor Saunders <[hidden email]> writes:

Trevor> isn't everything in -Wunused enabled now? can we just delete
Trevor> -Wno-unused and use -Wall to get us -Wunused?

I didn't think of that -- thanks.  From the gcc docs I see
-Wunused-label, -Wunused-local-typedefs, and -Wunused-parameter.  That
final one seems difficult for gdb given the many functions that are
called via function pointers but which do not use all their arguments.

Once the switch to C++ is complete, such parameters could be nameless.
To me that seems better than sticking ATTRIBUTE_UNUSED in many places.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 6/6] Add -Wunused-but-set-* to build

Trevor Saunders
On Tue, Jun 07, 2016 at 08:46:35PM -0600, Tom Tromey wrote:
> >>>>> "Trevor" == Trevor Saunders <[hidden email]> writes:
>
> Trevor> isn't everything in -Wunused enabled now? can we just delete
> Trevor> -Wno-unused and use -Wall to get us -Wunused?
>
> I didn't think of that -- thanks.  From the gcc docs I see
> -Wunused-label, -Wunused-local-typedefs, and -Wunused-parameter.  That
> final one seems difficult for gdb given the many functions that are
> called via function pointers but which do not use all their arguments.

the docs for -Wunused-parameter are... tricky, but I believe what they
say is -Wall -Wunused does not enable -Wunused-parameter, to enable
-Wunused-parameter you either need to pass it explicitly, or pass
-Wextra -Wunused.

> Once the switch to C++ is complete, such parameters could be nameless.
> To me that seems better than sticking ATTRIBUTE_UNUSED in many places.

I'd agree, and at that point it might make sense to enable
-Wunused-parameter.

Trev

>
> Tom
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 6/6] Add -Wunused-but-set-* to build

Tom Tromey-2
Trevor> the docs for -Wunused-parameter are... tricky, but I believe what they
Trevor> say is -Wall -Wunused does not enable -Wunused-parameter, to enable
Trevor> -Wunused-parameter you either need to pass it explicitly, or pass
Trevor> -Wextra -Wunused.

Yes, I see.  I think I misread them.  They say:

    '-Wunused'
         All the above '-Wunused' options combined.

         In order to get a warning about an unused function parameter, you
         must either specify '-Wextra -Wunused' (note that '-Wall' implies
         '-Wunused'), or separately specify '-Wunused-parameter'.

The first line implies that -Wunused-parameter is part of this, but then
subsequent text says not.

I'll give it a try and see what happens.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 6/6] Add -Wunused-but-set-* to build

Tom Tromey-2
Tom> I'll give it a try and see what happens.

It turns out that -Wunused-variable causes a number of errors (104 in my
build).  So, I think that while this change would be worthwhile, it's a
bit too much for me at present.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 6/6] Add -Wunused-but-set-* to build

Trevor Saunders
On Tue, Jun 07, 2016 at 10:15:59PM -0600, Tom Tromey wrote:
> Tom> I'll give it a try and see what happens.
>
> It turns out that -Wunused-variable causes a number of errors (104 in my
> build).  So, I think that while this change would be worthwhile, it's a
> bit too much for me at present.

Sure, I'll probably get to it in a couple weeks if you don't get there
first (I eventually want to merge bfd/warnings.m4 and gdb/warnings.m4)

Trev

>
> Tom
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 1/6] Change reopen_exec_file to check result of stat

Yao Qi
In reply to this post by Tom Tromey-2
On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <[hidden email]> wrote:
> This seems to be a real bug found by -Wunused-but-set-variable.  If
> "stat" fails for some reason, gdb would use the uninitialized "st".
>
> 2016-06-06  Tom Tromey  <[hidden email]>
>
>         * corefile.c (reopen_exec_file): Only examine st.st_mtime if stat
>         succeeded.

Patch is good to me.

--
Yao (齐尧)
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 3/6] Comment out some unused overlay code

Yao Qi
In reply to this post by Tom Tromey-2
On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <[hidden email]> wrote:
> This patch comments out some unneeded initializations in overlay code
> in symfile.c.  Normally I would not comment out code like this, but
> the uses of "size" in these functions are commented out.  If you'd
> prefer I could just delete the comments.

I am inclined to remove these commented-out statements.

These statements were commented out in 1998, and nobody wanted to add
them back again since then,

Fri Jan 23 07:47:06 1998  Fred Fish  <[hidden email]>

        * config/d10v/tm-d10v.h (CALL_DUMMY): Define as "{ 0 }".
        (TARGET_READ_FP): Define to d10v_read_fp rather than d10v_read_sp.
        (TARGET_WRITE_FP): Define to d10v_write_fp rather than d10v_write_sp.
        (d10v_write_fp, d10v_read_fp): Add prototypes.
        * symtab.c (decode_line_1): Remove assignment of sals[0].pc field.
        * symfile.c (simple_overlay_update, simple_overlay_update_1):
        Ignore the size of overlay sections.  This check is redundant anyway.

--
Yao (齐尧)
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 4/6] Remove some variables but call functions for side effects

Yao Qi
In reply to this post by Tom Tromey-2
On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <[hidden email]> wrote:

> This patch consolidates the (possibly-questionable) spots where we
> remove a declaration but continue to call some function for side
> effects.  In a couple of cases it wasn't entirely clear to me that
> this mattered; and in some other cases it might be more aesthetically
> pleasing to use ATTRIBUTE_UNUSED.  So, I broke this out into a
> separate patch for simpler review.
>
> 2016-06-06  Tom Tromey  <[hidden email]>
>
>         * arch-utils.c (default_skip_permanent_breakpoint): Remove
>         "bp_insn".
>         * disasm.c (do_assembly_only): Remove "num_displayed".
>         * dwarf2read.c (read_abbrev_offset): Remove "length".
>         (dwarf_decode_macro_bytes) <DW_MACINFO_vendor_ext>: Remove
>         "constant".
>         * m32c-tdep.c (make_regs): Remove "r2hl", "r3hl", and "intbhl".
>         * microblaze-tdep.c (microblaze_frame_cache): Remove "func".
>         * tracefile.c (trace_save): Remove "status".

Patch is good to me.

--
Yao (齐尧)
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 5/6] Remove unused variables

Yao Qi
In reply to this post by Tom Tromey-2
On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <[hidden email]> wrote:

> diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
> index aa25911..b663f50 100644
> --- a/gdb/python/py-framefilter.c
> +++ b/gdb/python/py-framefilter.c
> @@ -211,13 +211,11 @@ py_print_type (struct ui_out *out, struct value *val)
>
>    TRY
>      {
> -      struct type *type;
>        struct ui_file *stb;
>        struct cleanup *cleanup;
>
>        stb = mem_fileopen ();
>        cleanup = make_cleanup_ui_file_delete (stb);
> -      type = check_typedef (value_type (val));
>        type_print (value_type (val), "", stb, -1);
>        ui_out_field_stream (out, "type", stb);
>        do_cleanups (cleanup);

check_typedef has side effects, IIUC, so we can't remove the call to it.

--
Yao (齐尧)
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

Yao Qi
In reply to this post by Tom Tromey-2
On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <[hidden email]> wrote:
> I built and regression tested this using --enable-targets=all on
> x86-64 Fedora 23.  However, I could not update the various nat-*
> files, so there are probably unfixed warnings lurking there.

Can you use cross compiler to cross build native gdb to catch these warnings?
otherwise, once this patch series go in, gdb build on host other than linux may
be broken.

--
Yao (齐尧)
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 2/6] Use ATTRIBUTE_UNUSED in some places

Pedro Alves-7
In reply to this post by Tom Tromey-2
On 06/06/2016 10:33 PM, Tom Tromey wrote:
> A few spots needed ATTRIBUTE_UNUSED to cope with the new warnings.
>
> In the TUI this happens due to the odd way that "getyx" works -- it
> essentially uses references, so a variable must be passed in, even if
> the code doesn't need to use it.

I think we should use getcurx/getcury instead.

>
> The case in inflow.c is just a mass of ifdefs; and while the only use
> of "result" is guarded by "#if 0", I thought it simplest to leave it
> all in place.
>

Agreed.

Thanks,
Pedro Alves

Reply | Threaded
Open this post in threaded view
|

Re: [RFA 3/6] Comment out some unused overlay code

Tom Tromey-2
In reply to this post by Yao Qi
>>>>> "Yao" == Yao Qi <[hidden email]> writes:

Yao> I am inclined to remove these commented-out statements.

Sounds good.  Here's the new patch.

Tom

commit 8c3dd5d738cbb40684d1210392ae540799906a84
Author: Tom Tromey <[hidden email]>
Date:   Mon Jun 6 13:45:59 2016 -0600

    Remove some unused overlay code
   
    This patch removes some unneeded initializations in overlay code in
    symfile.c.  It also deletes some old commented-out code.
   
    2016-06-06  Tom Tromey  <[hidden email]>
   
    * symfile.c (simple_overlay_update_1): Remove initialization
    of "size", and commented-out code.
    (simple_overlay_update): Likewise.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index eacc3fa..ab545c8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2016-06-06  Tom Tromey  <[hidden email]>
+
+ * symfile.c (simple_overlay_update_1): Remove initialization
+ of "size", and commented-out code.
+ (simple_overlay_update): Likewise.
+
 2016-06-28  Tom Tromey  <[hidden email]>
 
  * tui/tui-winsource.c (tui_show_source_line): Use getcurx.
diff --git a/gdb/symfile.c b/gdb/symfile.c
index d29e96c..7d7843e 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3584,25 +3584,22 @@ simple_read_overlay_table (void)
 static int
 simple_overlay_update_1 (struct obj_section *osect)
 {
-  int i, size;
+  int i;
   bfd *obfd = osect->objfile->obfd;
   asection *bsect = osect->the_bfd_section;
   struct gdbarch *gdbarch = get_objfile_arch (osect->objfile);
   int word_size = gdbarch_long_bit (gdbarch) / TARGET_CHAR_BIT;
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
-  size = bfd_get_section_size (osect->the_bfd_section);
   for (i = 0; i < cache_novlys; i++)
     if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
- && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
- /* && cache_ovly_table[i][OSIZE] == size */ )
+ && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect))
       {
  read_target_long_array (cache_ovly_table_base + i * word_size,
  (unsigned int *) cache_ovly_table[i],
  4, word_size, byte_order);
  if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
-    && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
-    /* && cache_ovly_table[i][OSIZE] == size */ )
+    && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect))
   {
     osect->ovly_mapped = cache_ovly_table[i][MAPPED];
     return 1;
@@ -3660,15 +3657,13 @@ simple_overlay_update (struct obj_section *osect)
   ALL_OBJSECTIONS (objfile, osect)
     if (section_is_overlay (osect))
     {
-      int i, size;
+      int i;
       bfd *obfd = osect->objfile->obfd;
       asection *bsect = osect->the_bfd_section;
 
-      size = bfd_get_section_size (bsect);
       for (i = 0; i < cache_novlys; i++)
  if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
-    && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
-    /* && cache_ovly_table[i][OSIZE] == size */ )
+    && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect))
   { /* obj_section matches i'th entry in ovly_table.  */
     osect->ovly_mapped = cache_ovly_table[i][MAPPED];
     break; /* finished with inner for loop: break out.  */
123