This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Refactor tdep-i386.c to fix all -Wshadow warnings
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