Re: [RFC/PATCH] Clean up unused variables (and prepare for `-Wunused-variable' flag)

[Pedro Alves <palves@redhat.com>]

On 05/02/2012 06:30 AM, Sergio Durigan Junior wrote:

> Ok, sorry for taking so long to respond, this patch is huge and I was
> doing something else...
> 
> Well, here's the "obvious" patch that I came up with.  It contains only
> declarations of variables, not assignments to them.


No it doesn't.  ;-)

> 
> I know it's "obvious", but I prefer to ask than to apologize, so: is it
> OK to apply?
> 
> Of course, if you really want some rationale to the changes below, I
> will need to dive into the code and see why those variables are not
> used.
> 
> OK to apply?


> index aedda41..a7b67c5 100644
> --- a/gdb/gcore.c
> +++ b/gdb/gcore.c
> @@ -472,13 +472,12 @@ objfile_find_memory_regions (find_memory_region_ftype func, void *obfd)
>    /* Call callback function for each objfile section.  */
>    ALL_OBJSECTIONS (objfile, objsec)
>      {
> -      bfd *ibfd = objfile->obfd;
>        asection *isec = objsec->the_bfd_section;
> -      flagword flags = bfd_get_section_flags (ibfd, isec);
> +      flagword flags = bfd_get_section_flags (objfile->obfd, isec);
>
>        if ((flags & SEC_ALLOC) || (flags & SEC_LOAD))
>  	{
> -	  int size = bfd_section_size (ibfd, isec);
> +	  int size = bfd_section_size (objfile->obfd, isec);
>  	  int ret;
>
>  	  ret = (*func) (obj_section_addr (objsec), size,

As I've pointed out before, this needs a rationale.  (IMO, we should fix
the macro).  Several other places in the patch do this.

On 05/02/2012 06:30 AM, Sergio Durigan Junior wrote:
> @@ -238,8 +237,12 @@ m32r_supply_register (struct regcache *regcache, char *regname,
>        monitor_supply_register (regcache, regno, val);
>        if (regno == PSW_REGNUM)
>  	{
> +#if defined SM_REGNUM || defined BSM_REGNUM || defined IE_REGNUM \
> +    || defined BIE_REGNUM || defined COND_REGNUM  || defined CBR_REGNUM \
> +    || defined BPC_REGNUM || defined BCARRY_REGNUM
>  	  unsigned long psw = strtoul (val, NULL, 16);
>  	  char *zero = "00000000", *one = "00000001";
> +#endif

This doesn't fit in the category, but okay.  :-)

Could you wrap the multiline predicate in parenthesis though, please?

#if (defined SM_REGNUM || defined BSM_REGNUM || defined IE_REGNUM \
    || defined ...)


The rest looked good to me.

-- 
Pedro Alves