[KJ] [PATCH] mm: Clean up local variables

[KJ] [PATCH] mm: Clean up local variables

[Tobias Klauser]

[-- Attachment #1: Type: text/plain, Size: 2056 bytes --]

Clean up local variables with the same name as a variable in a larger
block. See http://linuxicc.sourceforge.net/Remark_1599.html for details.

Spotted by http://linuxicc.sourceforge.net/

Signed-off-by: Tobias Klauser <tklauser@nuerscht.ch>

---

 mm/slab.c     |    1 -
 mm/swapfile.c |   12 ++++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff -urpN -X dontdiff linux-2.6.15/mm/slab.c linux-2.6.15~tk/mm/slab.c
--- linux-2.6.15/mm/slab.c	2006-01-03 14:41:57.000000000 +0100
+++ linux-2.6.15~tk/mm/slab.c	2006-01-04 15:52:41.000000000 +0100
@@ -921,7 +921,6 @@ static int __devinit cpuup_callback(stru
 		down(&cache_chain_sem);
 
 		list_for_each_entry(cachep, &cache_chain, next) {
-			struct array_cache *nc;
 			cpumask_t mask;
 
 			mask = node_to_cpumask(node);
diff -urpN -X dontdiff linux-2.6.15/mm/swapfile.c linux-2.6.15~tk/mm/swapfile.c
--- linux-2.6.15/mm/swapfile.c	2006-01-03 14:41:57.000000000 +0100
+++ linux-2.6.15~tk/mm/swapfile.c	2006-01-04 15:59:20.000000000 +0100
@@ -1473,7 +1473,7 @@ asmlinkage long sys_swapon(const char __
 			goto bad_swap;
 		if (swap_header->info.nr_badpages > MAX_SWAP_BADPAGES)
 			goto bad_swap;
-		
+
 		/* OK, set up the swap map and apply the bad block list */
 		if (!(p->swap_map = vmalloc(maxpages * sizeof(short)))) {
 			error = -ENOMEM;
@@ -1482,17 +1482,17 @@ asmlinkage long sys_swapon(const char __
 
 		error = 0;
 		memset(p->swap_map, 0, maxpages * sizeof(short));
-		for (i=0; i<swap_header->info.nr_badpages; i++) {
-			int page = swap_header->info.badpages[i];
-			if (page <= 0 || page >= swap_header->info.last_page)
+		for (i = 0; i < swap_header->info.nr_badpages; i++) {
+			int page_nr = swap_header->info.badpages[i];
+			if (page_nr <= 0 || page_nr >= swap_header->info.last_page)
 				error = -EINVAL;
 			else
-				p->swap_map[page] = SWAP_MAP_BAD;
+				p->swap_map[page_nr] = SWAP_MAP_BAD;
 		}
 		nr_good_pages = swap_header->info.last_page -
 				swap_header->info.nr_badpages -
 				1 /* header page */;
-		if (error) 
+		if (error)
 			goto bad_swap;
 	}
 

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] mm: Clean up local variables

[Jesper Juhl]

On 1/4/06, Tobias Klauser <tklauser@nuerscht.ch> wrote:
> Clean up local variables with the same name as a variable in a larger
> block. See http://linuxicc.sourceforge.net/Remark_1599.html for details.
>
If you want to find a lot more of these, then add -Wshadow to the gcc
options in your kernels Makefile.

I tried submitting a patch that cleaned up a few to "test the waters"
as to making the kernel -Wshadow clean, but the patch had a mixed
reception. Good luck though, personally I'd like to see us be able to
add -Wshadow to the standard build flags - we just have to clean it
all up first and getting the patches merged looks like it could
possibly be difficult.

--
Jesper Juhl <jesper.juhl@gmail.com>
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

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] mm: Clean up local variables

[Matthew Wilcox]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

On Wed, Jan 04, 2006 at 06:01:19PM +0100, Tobias Klauser wrote:
> diff -urpN -X dontdiff linux-2.6.15/mm/slab.c linux-2.6.15~tk/mm/slab.c
> --- linux-2.6.15/mm/slab.c	2006-01-03 14:41:57.000000000 +0100
> +++ linux-2.6.15~tk/mm/slab.c	2006-01-04 15:52:41.000000000 +0100
> @@ -921,7 +921,6 @@ static int __devinit cpuup_callback(stru
>  		down(&cache_chain_sem);
>  
>  		list_for_each_entry(cachep, &cache_chain, next) {
> -			struct array_cache *nc;
>  			cpumask_t mask;
>  
>  			mask = node_to_cpumask(node);

While this does work, it's quite bad style.  Much better to move the
upper level declaration (line 856) into the block it's used in (line
893).  BTW, that function is too big at 133 lines and should be split.


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] mm: Clean up local variables

[Tobias Klauser]

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]

On 2006-01-04 at 18:18:19 +0100, Matthew Wilcox <matthew@wil.cx> wrote:
> On Wed, Jan 04, 2006 at 06:01:19PM +0100, Tobias Klauser wrote:
> > diff -urpN -X dontdiff linux-2.6.15/mm/slab.c linux-2.6.15~tk/mm/slab.c
> > --- linux-2.6.15/mm/slab.c	2006-01-03 14:41:57.000000000 +0100
> > +++ linux-2.6.15~tk/mm/slab.c	2006-01-04 15:52:41.000000000 +0100
> > @@ -921,7 +921,6 @@ static int __devinit cpuup_callback(stru
> >  		down(&cache_chain_sem);
> >  
> >  		list_for_each_entry(cachep, &cache_chain, next) {
> > -			struct array_cache *nc;
> >  			cpumask_t mask;
> >  
> >  			mask = node_to_cpumask(node);
> 
> While this does work, it's quite bad style.  Much better to move the
> upper level declaration (line 856) into the block it's used in (line
> 893).  BTW, that function is too big at 133 lines and should be split.

Sure. That makes more sense. I'll send a new patch.

Thanks, Tobias

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors