Re: [PATCH] remove trivialy unused variables

[Pedro Alves <palves at redhat dot com>]

On 04/29/2016 11:55 PM, tbsaunde+binutils@tbsaunde.org wrote:
> From: Trevor Saunders <tbsaunde+binutils@tbsaunde.org>
> 
> Hi,

Hi Trevor, thanks for doing this.

I've read the patch, and it looks mostly OK to me, though I think
I may have spotted one problem.

> 
> I thought I'd see how hard it is to enable -Wunused for gdb.  There's plenty
> more warnings, but this is already a large patch.  I think all these are cases
> where the variable is clearly useless and getting rid of it is an improvement,
> the really nice part is this lets us get rid of a number of function calls.
> 
> I did a --enable-targets=all build successfully, ok?

We'll need to run this patch against the testsuite with both a native target:

 $ make check -jN

and gdbserver, to cover tracing:

 $ make check-parallel -jN RUNTESTFLAGS="--target_board=native-gdbserver"

See why below.

> @@ -1843,7 +1826,6 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
>  		      struct type *valtype, struct regcache *regcache,
>  		      gdb_byte *readbuf, const gdb_byte *writebuf)
>  {
> -  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>  
>    if (TYPE_CODE (valtype) == TYPE_CODE_STRUCT

Remove empty line.

>        || TYPE_CODE (valtype) == TYPE_CODE_UNION

> @@ -11136,8 +11119,6 @@ queue_and_load_all_dwo_tus (struct dwarf2_per_cu_data *per_cu)
>  static void
>  free_dwo_file (struct dwo_file *dwo_file, struct objfile *objfile)
>  {
> -  int ix;
> -  struct dwarf2_section_info *section;
>  
>    /* Note: dbfd is NULL for virtual DWO files.  */


Ditto.

>    gdb_bfd_unref (dwo_file->dbfd);
> @@ -12340,7 +12321,6 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>  static void
>  check_producer (struct dwarf2_cu *cu)
>  {
> -  const char *cs;
>    int major, minor;
>  
>    if (cu->producer == NULL)
> @@ -12640,11 +12620,8 @@ static void

> --- a/gdb/m32r-linux-tdep.c
> +++ b/gdb/m32r-linux-tdep.c
> @@ -447,7 +447,6 @@ m32r_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
>  static void
>  m32r_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
> -  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>  
>    linux_init_abi (info, gdbarch);

Ditto.

> --- a/gdb/ravenscar-thread.c
> +++ b/gdb/ravenscar-thread.c
> @@ -338,7 +338,6 @@ ravenscar_mourn_inferior (struct target_ops *ops)
>  static void
>  ravenscar_inferior_created (struct target_ops *target, int from_tty)
>  {
> -  struct ravenscar_arch_ops *ops;
>  
>    if (!ravenscar_task_support

Ditto.

>        || gdbarch_ravenscar_ops (target_gdbarch ()) == NULL



> @@ -77,10 +75,6 @@ trace_save (const char *filename, struct trace_file_writer *writer,
>        return;
>      }
>  
> -  /* 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);
> -

I believe this call is needed, since this is what fills in 'ts'
in the first place.



The patch also removes a couple calls to check_typedef that might
have been there for side effects of check_typedef.
We used to have a CHECK_TYPEDEF macro that was eliminated by
expanding it and I thought that might have been what introduced the
unused type variables:

 https://sourceware.org/ml/gdb-patches/2015-07/msg00146.html

though it doesn't look that's been the case here.  So if testing doesn't
reveal any check_typedef problem, I'm happy with those removals too.

Thanks,
Pedro Alves