Re: Refactor tdep-i386.c to fix all -Wshadow warnings

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Sunday, August 05 2012, Goncalo Gomes wrote:

> The attached patch fixes all warnings in tdep-i386.c resulting from
> enabling shadow warnings in gcc. As this is my first contribution to
> GDB, I decided to keep it short to a single file to obtain feedback.

Thanks for your patch.  It is a cleanup and not a big patch, thus I
guess you won't need a copyright assignment to get it in.  However, if
you would like to start the process, drop me an e-mail offlist and I can
send you the form.

I have a few comments regarding formatting nits.

> 2012-08-05  Goncalo Gomes <goncalo@promisc.org>

Two spaces between your name and your e-mail address.

>
>         * Refactor tdep-i386.c to fix all -Wshadow warnings

You have to explicit mention each file, and each function/location being
modified.  Take a look at gdb/ChangeLog to see how it is done.  You also
need to indent the entries using a tab, and not spaces.  Something like:

2012-08-06  Goncalo Gomes  <goncalo@promisc.org>

	* exceptions.h (TRY_CATCH): Rename `buf' to `trycatch_sjbuf'.
	* i386-tdep.c (....)...

> diff --git a/gdb/exceptions.h b/gdb/exceptions.h
> index 0c59a25..9c47a7b 100644
> --- a/gdb/exceptions.h
> +++ b/gdb/exceptions.h
> @@ -145,9 +145,9 @@ int exceptions_state_mc_action_iter_1 (void);
>  
>  #define TRY_CATCH(EXCEPTION,MASK) \
>       { \
> -       EXCEPTIONS_SIGJMP_BUF *buf = \
> +       EXCEPTIONS_SIGJMP_BUF *_trycatch_sjbuf = \
>  	 exceptions_state_mc_init (&(EXCEPTION), (MASK)); \
> -       EXCEPTIONS_SIGSETJMP (*buf); \
> +       EXCEPTIONS_SIGSETJMP (*_trycatch_sjbuf); \
>       } \
>       while (exceptions_state_mc_action_iter ()) \
>         while (exceptions_state_mc_action_iter_1 ())

Is it possible to remove the leading `_' from `_trycatch_sjbuf'?  It
should be used only for internal variables inside some library which I
forgot the name...

> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 84e9794..6fbc51f 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c

> @@ -3561,7 +3561,7 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
>  		  const char *start;
>  		  char *base;
>  		  int len_base;
> -		  char *index;
> +		  char *idx;
>  		  int len_index;

Please change this variable to `len_idx', since it refers to `idx' now.

Aside of that, I guess the patch is pretty trivial.  I am not a
maintainer, so you will still need an approval from a maintainer before
being able to check it in.  In fact, I guess you won't be able to check
it in for yourself because you still don't have an account on
sourceware, so probably someone else will commit it for you.

Thanks,

-- 
Sergio