Hi,
Variables shadowing eachother have a nasty potential to contain hard to
discover bugs, so I propose we clean that up.
There are *lots* of cases of variable shadowing in the kernel and real bugs
could be lurking in there.
I'm full willing to audit every single case and clean them up, but before I
begin such a task I want to "test the waters".
So to get things started I give you the patch below that cleans up
784 counts of
include/linux/jiffies.h:331: warning: declaration of 'jiffies' shadows a global declaration
784 counts of
include/linux/jiffies.h:369: warning: declaration of 'jiffies' shadows a global declaration
30 counts of
include/linux/usb.h:877: warning: declaration of 'complete' shadows a global declaration
30 counts of
include/linux/usb.h:908: warning: declaration of 'complete' shadows a global declaration
30 counts of
include/linux/usb.h:943: warning: declaration of 'complete' shadows a global declaration
1 count of
mm/swap.c:42: warning: declaration of 'page' shadows a parameter
So with just one simple patch we've nuked 1659 -Wshadow warnings out of a total
of 2403 that this build of mine generated with my regular .config
Of course I picked some of the easy targets to start with, a lot of the
remaining ones are cases of 1-10 identical warnings generated by one instance.
Anyway, I think this is a worthwhile task and I'm willing to do it and we may
even discover/fix real bugs along the way, so what do you say?
Comments are very welcome as always, and feel free to merge this patch as a
starting point if you agree that this is something that ought to be done.
Signed-off-by: Jesper Juhl <[email protected]>
---
include/linux/jiffies.h | 8 ++++----
include/linux/usb.h | 12 ++++++------
mm/swap.c | 2 +-
3 files changed, 11 insertions(+), 11 deletions(-)
--- linux-2.6.15-rc5-git4-orig/include/linux/jiffies.h 2005-10-28 02:02:08.000000000 +0200
+++ linux-2.6.15-rc5-git4/include/linux/jiffies.h 2005-12-14 22:52:07.000000000 +0100
@@ -328,13 +328,13 @@ timespec_to_jiffies(const struct timespe
}
static __inline__ void
-jiffies_to_timespec(const unsigned long jiffies, struct timespec *value)
+jiffies_to_timespec(const unsigned long jiff, struct timespec *value)
{
/*
* Convert jiffies to nanoseconds and separate with
* one divide.
*/
- u64 nsec = (u64)jiffies * TICK_NSEC;
+ u64 nsec = (u64)jiff * TICK_NSEC;
value->tv_sec = div_long_long_rem(nsec, NSEC_PER_SEC, &value->tv_nsec);
}
@@ -366,13 +366,13 @@ timeval_to_jiffies(const struct timeval
}
static __inline__ void
-jiffies_to_timeval(const unsigned long jiffies, struct timeval *value)
+jiffies_to_timeval(const unsigned long jiff, struct timeval *value)
{
/*
* Convert jiffies to nanoseconds and separate with
* one divide.
*/
- u64 nsec = (u64)jiffies * TICK_NSEC;
+ u64 nsec = (u64)jiff * TICK_NSEC;
value->tv_sec = div_long_long_rem(nsec, NSEC_PER_SEC, &value->tv_usec);
value->tv_usec /= NSEC_PER_USEC;
}
--- linux-2.6.15-rc5-git4-orig/include/linux/usb.h 2005-12-04 18:48:51.000000000 +0100
+++ linux-2.6.15-rc5-git4/include/linux/usb.h 2005-12-14 23:57:09.000000000 +0100
@@ -874,7 +874,7 @@ static inline void usb_fill_control_urb
unsigned char *setup_packet,
void *transfer_buffer,
int buffer_length,
- usb_complete_t complete,
+ usb_complete_t comp,
void *context)
{
spin_lock_init(&urb->lock);
@@ -883,7 +883,7 @@ static inline void usb_fill_control_urb
urb->setup_packet = setup_packet;
urb->transfer_buffer = transfer_buffer;
urb->transfer_buffer_length = buffer_length;
- urb->complete = complete;
+ urb->complete = comp;
urb->context = context;
}
@@ -905,7 +905,7 @@ static inline void usb_fill_bulk_urb (st
unsigned int pipe,
void *transfer_buffer,
int buffer_length,
- usb_complete_t complete,
+ usb_complete_t comp,
void *context)
{
spin_lock_init(&urb->lock);
@@ -913,7 +913,7 @@ static inline void usb_fill_bulk_urb (st
urb->pipe = pipe;
urb->transfer_buffer = transfer_buffer;
urb->transfer_buffer_length = buffer_length;
- urb->complete = complete;
+ urb->complete = comp;
urb->context = context;
}
@@ -940,7 +940,7 @@ static inline void usb_fill_int_urb (str
unsigned int pipe,
void *transfer_buffer,
int buffer_length,
- usb_complete_t complete,
+ usb_complete_t comp,
void *context,
int interval)
{
@@ -949,7 +949,7 @@ static inline void usb_fill_int_urb (str
urb->pipe = pipe;
urb->transfer_buffer = transfer_buffer;
urb->transfer_buffer_length = buffer_length;
- urb->complete = complete;
+ urb->complete = comp;
urb->context = context;
if (dev->speed == USB_SPEED_HIGH)
urb->interval = 1 << (interval - 1);
--- linux-2.6.15-rc5-git4-orig/mm/swap.c 2005-12-04 18:48:54.000000000 +0100
+++ linux-2.6.15-rc5-git4/mm/swap.c 2005-12-14 23:04:28.000000000 +0100
@@ -39,7 +39,7 @@ void put_page(struct page *page)
if (unlikely(PageCompound(page))) {
page = (struct page *)page_private(page);
if (put_page_testzero(page)) {
- void (*dtor)(struct page *page);
+ void (*dtor)(struct page *pge);
dtor = (void (*)(struct page *))page[1].mapping;
(*dtor)(page);
On Thu, Dec 15, 2005 at 12:19:57AM +0100, Jesper Juhl wrote:
> - void (*dtor)(struct page *page);
> + void (*dtor)(struct page *pge);
Note that this one just needs to be:
void (*dtor)(struct page *);
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On 12/15/05, Russell King <[email protected]> wrote:
> On Thu, Dec 15, 2005 at 12:19:57AM +0100, Jesper Juhl wrote:
> > - void (*dtor)(struct page *page);
> > + void (*dtor)(struct page *pge);
>
> Note that this one just needs to be:
> void (*dtor)(struct page *);
>
Yes, I considered just doing that, but was unsure if the name had been
used for some obscure readability or grep'ability reason, so I opted
to leave the name but just change it.. In future cases like this one
(if people actually want these patches) I'll just nuke the name.
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
On Wed, 14 Dec 2005, Russell King wrote:
> On Thu, Dec 15, 2005 at 12:19:57AM +0100, Jesper Juhl wrote:
> > - void (*dtor)(struct page *page);
> > + void (*dtor)(struct page *pge);
>
> Note that this one just needs to be:
> void (*dtor)(struct page *);
I would rather see <shadowed> names there.
--
~Randy