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

[Goncalo Gomes <goncalo@promisc.org>]

On Mon, Aug 6, 2012 at 11:46 AM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> 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.
>
(...)
> 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 for reviewing. I have (hopefully) applied all your suggestions.

I'm going to re-send the revised patch and changelog in a follow-up
email with a new subject line.

Goncalo