making the kernel -Wshadow clean - The initial step
Subsystems (14)
cover making the kernel -Wshadow clean - The initial step
Ok, here we go again...
This is a series of patches that try to be an initial step towards making the kernel build -Wshadow clean.
Yeah, I'm persisting with this since I get the impression, from the feedback I've been getting, that this effort is mostly looked upon favorably.
The reasons why we should do this should be obvious, but I'll list the main reasons anyway.
In the end the compiler will help us prevent new (and hard to find bugs) due to symbol shadowing.
It'll help us keep our namespaces separate.
We may find some bugs along the way - and even if it's just one it'll be worth it :)
I have recieved a lot of useful feedback to the previous patches I've posted and I've tried to incorporate all the feedback in this series.
There should be no change in behaviour from these patches and it's my hope that they are acceptable for merging so that, going forward, I will have these 12 less to keep track of separately.
A patch to add -Wshadow to the toplevel Makefile is not included since it's still a little early for that. There are still a lot of warnings that need to be fixed, this series is just the first step (but as I think the numbers below will show, it's a pretty big first step).
I'm only able to work against i386 at the moment, so the results below probably won't be as nice for other archs, but no need to worry, I do plan to clean up other archs as well - I just need to get a bunch of cross-compilers installed.
My plan is to keep submitting new series of patches against -mm (assuming that these will get merged there) until we are down to less than 100 warnings for all the standard configs for all archs and then at that point submit a patch to add -Wshadow to the toplevel Makefile - if someone wants to help out I'd appreciate it - there are still lots of warnings left to work on, some of them I've just not gotten to yet, but some of them I'll definately need help with as well.
All of this is against 2.6.18-rc3.
Without the patches, adding -Wshadow to the makefile results in these warnings for a few different .config's :
allnoconfig
---
$ grep warning build.log.allno | grep shadow | wc -l 1267
allmodconfig
---
$ grep warning build.log.allmod | grep shadow | wc -l 39921
allyesconfig
---
$ grep warning build.log.allyes | grep shadow | wc -l 32477
with my normal workstation config
---
$ grep warning build.log.juhl | grep shadow | wc -l 5298
After applying the patches in this series, quite a different picture emerges.
allnoconfig : 124 warnings (~10%)
allmodconfig : 6508 warnings (~16%)
allyesconfig : 6522 warnings (~20%)
my config : 958 warnings (~18%)
So with just 12 small patches the number of -Wshadow related warnings is reduced to between 10 and 20 percent of what they previously were. Personally I'd say that's a pretty good first step.
I've added a bunch of people to Cc: on this mail, trying to pick those that have either responded to my previous attempts or whoes code I'm changing. I may not have gotten it 100% right, so please bear with me. I'll not be sending the actual patches to all of you guys - don't want to spam your inbox too much. Instead I'll send all 12 patches as a reply to
this email to LKML with just akpm cc. I hope that's OK with everyone.
Patch series overview :
[PATCH 00/12] making the kernel -Wshadow clean - The initial step [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
[PATCH 02/12] making the kernel -Wshadow clean - fix lxdialog
[PATCH 03/12] making the kernel -Wshadow clean - fix jiffies.h
[PATCH 04/12] making the kernel -Wshadow clean - warnings related to 'up' [PATCH 05/12] making the kernel -Wshadow clean - warnings related to wbc and map_bh [PATCH 06/12] making the kernel -Wshadow clean - fix checksum
[PATCH 07/12] making the kernel -Wshadow clean - fix vgacon
[PATCH 08/12] making the kernel -Wshadow clean - fix keyboard.c
[PATCH 09/12] making the kernel -Wshadow clean - neighbour.h and 'proc_handler' [PATCH 10/12] making the kernel -Wshadow clean - mm/truncate.c
[PATCH 11/12] making the kernel -Wshadow clean - USB & completion [PATCH 12/12] making the kernel -Wshadow clean - 'irq' shadows local and global
Signature · 3 lines
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
PATCH 1/12 making the kernel -Wshadow clean - fix mconf
Fix -Wshadow warnings in mconf
---
scripts/kconfig/mconf.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
scripts/kconfig/mconf.c +3 -3
@@ -258,7 +258,7 @@ search_help[] = N_(static char buf[4096 ], *bufptr = buf;static char input_buf[4096 ]; -static char filename[PATH_MAX +1 ] =".config" ; +static char config_filename[PATH_MAX +1 ] =".config" ;static char *args[1024 ], **argptr = args;static int indent;static struct termios ios_org;
@@ -983,7 +983,7 @@ static void conf_load(void)cprint (load_config_text);cprint ("11" );cprint ("55" ); -cprint ("%s" , filename); +cprint ("%s" , config_filename); stat =exec_conf ();switch (stat) {case 0 :
@@ -1012,7 +1012,7 @@ static void conf_save(void)cprint (save_config_text);cprint ("11" );cprint ("55" ); -cprint ("%s" , filename); +cprint ("%s" , config_filename); stat =exec_conf ();switch (stat) {case 0 :
Re: [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
Review
Quoted 2 lines · Jesper
(quoted diff) +1
-1- cprint("%s", filename); + cprint("%s", config_filename);
Something seems strange here to me. It looks like you are sometimes resolving the shadowed symbols by making the more local symbol have the longer name.
I'd have expected that the global symbol would be the one with the longer, more elaborate name.
In other words, I would have expected that we would avoid having global names such as (from your other patches in this set):
filename
scroll
instr
up
sum
state
rep
complete
irq
Perhaps I am misreading this patch set?
Signature · 3 lines
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401Re: [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
Quoted 8 lines · Paul Jackson
Review
Quoted 2 lines · Jesper
(quoted diff) +1
-1- cprint("%s", filename); + cprint("%s", config_filename);Something seems strange here to me. It looks like you are sometimes resolving the shadowed symbols by making the more local symbol have the longer name.
True.
Quoted 3 lines
I'd have expected that the global symbol would be the one with the longer, more elaborate name.
Generally I'd agree with you, but my initial objective is to resolve all (or at least most) of the clashes with as little pain as pssible initially so that we can get to the point where we can add -Wshadow to the Makefile - sometimes the path of least resistence is making the local name longer.
Quoted 4 lines
In other words, I would have expected that we would avoid having global names such as (from your other patches in this set):
filename
Here I changed the global to be longer - config_filename.
Quoted 1 line
scroll
made the local longer - guilty as charged.
Quoted 1 line
instr
I don't recall using that variable name - I believe you mean 'intr' for interrupt that I used in place of 'irq'.
Quoted 1 line
up
I'd *love* to change this one - and down() as well - to up_sem() & down_sem(). But, making that change would be a pretty major and somewhat disruptive api change, so I opted instead to change the local variable names. I plan to introduce a sepperate patch set later on that adds up_sem()/down_sem() wrappers around up()/down(), deprecate the old names and add an entry to feature-removal.txt - but doing it now as part of the -Wshadow cleanup would be too much pain.
Quoted 6 lines
sum
state
rep
complete
irq
Yes, here I made the local name longer. Long term that should probably change. Short term it seemed the path of least resistance.
Quoted 2 lines
Perhaps I am misreading this patch set?
i don't think you are. It's just that I want to take the least intrusive route *now*, make us -Wshadow clean, get -Wshadow to be an accepted part of the Makefile, *then* deal with the more intrusive/controversial renamings, where I guess you'd have done things in the opposite order - right?
Signature · 3 lines
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
Re: [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
Quoted 7 lines · On Sun, 30 Jul 2006 20:48:23 +0200 "Jesper Juhl" <[email protected]> wrote:
Quoted 2 lines
Perhaps I am misreading this patch set?
i don't think you are. It's just that I want to take the least intrusive route *now*, make us -Wshadow clean, get -Wshadow to be an accepted part of the
Makefile, *then* deal with the more intrusive/controversial renamings, where I guess you'd have done things in the opposite order - right?
yup. Experience tells us that it's better to get things right first time, because we don't get around to doing the intended second pass (looks at lock_cpu_hotplug())
That being said, no, we can't go and rename up(). Let us go through the patches one-at-a-time..
Re: [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
Quoted 5 lines · Andrew Morton
Quoted 2 lines · On Sun, 30 Jul 2006 20:48:23 +0200 "Jesper Juhl" <[email protected]> wrote:
Quoted 2 lines
Perhaps I am misreading this patch set?
Quoted 8 lines
Quoted 5 lines
i don't think you are. It's just that I want to take the least intrusive route *now*, make us -Wshadow clean, get -Wshadow to be an accepted part of the
Makefile, *then* deal with the more intrusive/controversial renamings, where I guess you'd have done things in the opposite order - right?yup. Experience tells us that it's better to get things right first time, because we don't get around to doing the intended second pass
I believe my (modest) track record would show that I do follow up on these things...
But anyway, I believe the renames I've done are not bad one way or the other and should be acceptable (and some of the long names for local symbols are chosen based on maintainer feedback even)... but I'll await more feedback and change the patches if needed.
Quoted 3 lines
(looks at
lock_cpu_hotplug())
Hmm, I'll take a look at lock_cpu_hotplug() - can you provide additional details?
Quoted 3 lines
That being said, no, we can't go and rename
up(). Let us go through the patches one-at-a-time..
i figured as much. *But* won't you agree that up_sem() (or considering the other locking function names, sem_up() would probably be better) would be a much better name for a global like this one?
How about a plan like this:
We introduce sem_up() and sem_down() wrapper functions now. They could go into 2.6.19 for example - and also add a note to feature-removal-schedule.txt that the old function names will be removed in 1 year. Then in a few kernel versions - say 2.6.21 - we deprecate the old names and add a big fac comment in the source as well.
Wouldn't that be doable? Or do we have to live with up()/down() forever?
Signature · 3 lines
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
Re: [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
Quoted 6 lines · On Sun, 30 Jul 2006 21:17:18 +0200 "Jesper Juhl" <[email protected]> wrote:
Quoted 3 lines
(looks at
lock_cpu_hotplug())Hmm, I'll take a look at
lock_cpu_hotplug()- can you provide additional details?
eh. We put the recursive-sem thing in there as a temp fix to cpufreq to get 2.6.something out the door, expressing fine intentions to fix it for real later on. Then look what happened. Don't go there.
Quoted 8 lines
Quoted 4 lines
That being said, no, we can't go and rename
up(). Let us go through the patches one-at-a-time..i figured as much. *But* won't you agree that
up_sem()(or considering the other locking function names,sem_up()would probably be better) would be a much better name for a global like this one?
Quoted 8 lines
How about a plan like this:
We introducesem_up()andsem_down()wrapper functions now. They could go into 2.6.19 for example - and also add a note to feature-removal-schedule.txt that the old function names will be removed in 1 year. Then in a few kernel versions - say 2.6.21 - we deprecate the old names and add a big fac comment in the source as well.
Wouldn't that be doable? Or do we have to live with up()/down() forever?
Well actually when struct mutex went in we decided to remove all non-counting uses of semaphores kernel-wide, migrating them to mutexes. Then to remove all the arch-specific semaphore implementations and implement an arch-neutral version. After that has been done would be an appropriate time to rename things.
But it never happened. See "fine intentions", above ;)
Re: [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
Quoted 8 lines
Well actually when struct mutex went in we decided to remove all non-counting uses of semaphores kernel-wide, migrating them to mutexes. Then to remove all the arch-specific semaphore implementations and implement an arch-neutral version. After that has been done would be an appropriate time to rename things.
But it never happened. See "fine intentions", above ;)
it's still in progress ;)
even in 2.6.18-rc there are semaphore to mutex conversions.... the remaining ones are increasingly tricky though so speed is slowing down
Signature · 1 line
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Re: [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
In reply to Andrew MortonQuoted 6 lines · Andrew Morton
Quoted 3 lines · On Sun, 30 Jul 2006 21:17:18 +0200 "Jesper Juhl" <[email protected]> wrote:
Quoted 3 lines
(looks at
lock_cpu_hotplug())
Quoted 8 lines
Quoted 3 lines
Hmm, I'll take a look at
lock_cpu_hotplug()- can you provide additional details?eh. We put the recursive-sem thing in there as a temp fix to cpufreq to get 2.6.something out the door, expressing fine intentions to fix it for real later on. Then look what happened. Don't go there.
Ok, that's probably way over my head, but I'll dig in none the less and see what I can do to help. It'll probably land me in a world of hurt, but I've taken flames before and I'm still here ;-) Don't expect much, but I'll see if there's anything I can do at least.
Quoted 17 lines
Quoted 8 lines
Quoted 4 lines
That being said, no, we can't go and rename
up(). Let us go through the patches one-at-a-time..i figured as much. *But* won't you agree that
up_sem()(or considering the other locking function names,sem_up()would probably be better) would be a much better name for a global like this one?Quoted 8 lines
How about a plan like this:
We introducesem_up()andsem_down()wrapper functions now. They could go into 2.6.19 for example - and also add a note to feature-removal-schedule.txt that the old function names will be removed in 1 year. Then in a few kernel versions - say 2.6.21 - we deprecate the old names and add a big fac comment in the source as well.
Wouldn't that be doable? Or do we have to live with up()/down() forever?
Quoted 2 lines
Well actually when struct mutex went in we decided to remove all non-counting uses of semaphores kernel-wide, migrating them to mutexes.
Makes sense.
Quoted 4 lines
Then to remove all the arch-specific semaphore implementations and implement an arch-neutral version. After that has been done would be an appropriate time to rename things.
Ok, that is (again) probably beyond me, but I'll still take a look at it just for the hell of it.
If nothing else I can at least keep an eye out for when we reach the point we want to be at and then submit renaming patches... let's see..
Quoted 2 lines
But it never happened. See "fine intentions", above ;)
Heh, The road to hell is paved with fine intentions ;-)
Signature · 3 lines
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
Re: [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
In reply to Andrew MortonQuoted 1 line
we can't go and rename
up()
True - quite true.
Signature · 3 lines
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401Arnd Bergmann
maintainerRe: [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
In reply to Jesper JuhlAm Sunday 30 July 2006 20:48 schrieb Jesper Juhl:
Quoted 9 lines
Quoted 1 line
up
I'd *love* to change this one - and
down()as well - toup_sem()&down_sem(). But, making that change would be a pretty major and somewhat disruptive api change, so I opted instead to change the local variable names. I plan to introduce a sepperate patch set later on that adds up_sem()/down_sem() wrappers around up()/down(), deprecate the old names and add an entry to feature-removal.txt - but doing it now as part of the -Wshadow cleanup would be too much pain.
The path for getting rid of up()/down() is more along the lines of replacing more semaphores with mutex variables. Once the only users of up()/down() are those that really rely on counting semaphores, it becomes much easier to do the change you proposed.
Arnd <><
Re: [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
In reply to Jesper JuhlQuoted 8 lines · Jesper Juhl
Quoted 7 lines · Paul Jackson
Review
Quoted 2 lines · Jesper
(quoted diff) +1
-1- cprint("%s", filename); + cprint("%s", config_filename);Something seems strange here to me. It looks like you are sometimes resolving the shadowed symbols by making the more local symbol have the longer name.
[...]
Quoted 4 lines
Quoted 1 line
instr
I don't recall using that variable name - I believe you mean 'intr' for interrupt that I used in place of 'irq'.
Please don't. If people are accustomed to irq, they will start wondering what intr is all about (or what the difference is, etc).
Quoted 4 lines
Quoted 1 line
up
I'd *love* to change this one - and
down()as well - toup_sem()&down_sem().
Just too bad that there aren't semaphores anymore... and I can't find up() down() in the headers anyway?
Quoted 6 lines
But, making that change would be a pretty major and
somewhat disruptive api change, so I opted instead to change the local variable names. I plan to introduce a sepperate patch set later on that adds up_sem()/down_sem() wrappers around up()/down(), deprecate the old names and add an entry to feature-removal.txt - but doing it now as part of the -Wshadow cleanup would be too much pain.
Why not leave them alone for the time being then, and clean up in one sweep later on?
Signature · 4 lines
Dr. Horst H. von Brand User #22616 counter.li.org Departamento de Informatica Fono: +56 32 654431 Universidad Tecnica Federico Santa Maria +56 32 654239 Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513
Re: [PATCH 01/12] making the kernel -Wshadow clean - fix mconf
Quoted 8 lines · Horst H. von Brand
Quoted 4 lines
Quoted 2 lines
instr
I don't recall using that variable name - I believe you mean 'intr' for interrupt that I used in place of 'irq'.
Please don't. If people are accustomed to irq, they will start wondering what intr is all about (or what the difference is, etc).
Worse, on the x86 platform, people may very well assume that irq 0 = intr 32 etc.
-hpa
PATCH 2/12 making the kernel -Wshadow clean - fix lxdialog
In reply to Jesper JuhlRemove a bunch of -Wshadow warnings from scripts/kconfig/lxdialog/
scripts/kconfig/lxdialog/checklist.c | 70 ++++++++++----------- scripts/kconfig/lxdialog/dialog.h | 6 - scripts/kconfig/lxdialog/inputbox.c | 38 +++++------ scripts/kconfig/lxdialog/menubox.c | 82 ++++++++++++------------- scripts/kconfig/lxdialog/msgbox.c | 4 - scripts/kconfig/lxdialog/util.c | 20 +++--- 6 files changed, 110 insertions(+), 110 deletions(-)
scripts/kconfig/lxdialog/checklist.c +35 -35
@@ -56,12 +56,12 @@ static void print_item(WINDOW * win, con/* * Print the scroll indicators. */ -static void print_arrows (WINDOW * win,int choice,int item_no,int scroll, +static void print_arrows (WINDOW * win,int choice,int item_no,int scrolling,int y,int x,int height) {wmove (win, y, x); -if (scroll >0 ) { +if (scrolling >0 ) {wattrset (win, uarrow_attr);waddch (win,ACS_UARROW );waddstr (win,"(-)" );
@@ -76,7 +76,7 @@ static void print_arrows(WINDOW * win, i y = y + height +1 ;wmove (win, y, x); -if ((height < item_no) && (scroll + choice < item_no -1 )) { +if ((height < item_no) && (scrolling + choice < item_no -1 )) {wattrset (win, darrow_attr);waddch (win,ACS_DARROW );waddstr (win,"(+)" );
@@ -113,7 +113,7 @@ int dialog_checklist(const char *title, const char *const *items) {int i, x, y, box_x, box_y; -int key =0 , button =0 , choice =0 , scroll =0 , max_choice, *status; +int key =0 , button =0 , choice =0 , scrolling =0 , max_choice, *status;WINDOW *dialog, *list;/* Allocate space for storing item on/off status */
@@ -181,20 +181,20 @@ int dialog_checklist(const char *title, item_x = check_x +4 ;if (choice >= list_height) { - scroll = choice - list_height +1 ; - choice -= scroll; + scrolling = choice - list_height +1 ; + choice -= scrolling; }/* Print the list */ for (i =0 ; i < max_choice; i++) {if (i != choice) -print_item (list, items[(scroll + i) *3 +1 ], - status[i + scroll], i,0 ); +print_item (list, items[(scrolling + i) *3 +1 ], + status[i + scrolling], i,0 ); } -print_item (list, items[(scroll + choice) *3 +1 ], - status[choice + scroll], choice,1 ); +print_item (list, items[(scrolling + choice) *3 +1 ], + status[choice + scrolling], choice,1 ); -print_arrows (dialog, choice, item_no, scroll, +print_arrows (dialog, choice, item_no, scrolling, box_y, box_x + check_x +5 , list_height);print_buttons (dialog, height, width,0 );
@@ -208,28 +208,28 @@ int dialog_checklist(const char *title,for (i =0 ; i < max_choice; i++)if (toupper (key) == -toupper (items[(scroll + i) *3 +1 ][0 ])) +toupper (items[(scrolling + i) *3 +1 ][0 ]))break ;if (i < max_choice || key ==KEY_UP || key ==KEY_DOWN || key =='+' || key =='-' ) {if (key ==KEY_UP || key =='-' ) {if (!choice) { -if (!scroll) +if (!scrolling)continue ;/* Scroll list down */ if (list_height >1 ) {/* De-highlight current first item */ -print_item (list, items[scroll *3 +1 ], - status[scroll],0 , FALSE); +print_item (list, items[scrolling *3 +1 ], + status[scrolling],0 , FALSE);scrollok (list, TRUE);wscrl (list,-1 );scrollok (list, FALSE); } - scroll--; -print_item (list, items[scroll *3 +1 ], status[scroll],0 , TRUE); + scrolling--; +print_item (list, items[scrolling *3 +1 ], status[scrolling],0 , TRUE);print_arrows (dialog, choice, item_no, - scroll, box_y, box_x + check_x +5 , list_height); + scrolling, box_y, box_x + check_x +5 , list_height);wnoutrefresh (dialog);wrefresh (list);
@@ -239,24 +239,24 @@ int dialog_checklist(const char *title, i = choice -1 ; }else if (key ==KEY_DOWN || key =='+' ) {if (choice == max_choice -1 ) { -if (scroll + choice >= item_no -1 ) +if (scrolling + choice >= item_no -1 )continue ;/* Scroll list up */ if (list_height >1 ) {/* De-highlight current last item before scrolling up */ -print_item (list, items[(scroll + max_choice -1 ) *3 +1 ], - status[scroll + max_choice -1 ], +print_item (list, items[(scrolling + max_choice -1 ) *3 +1 ], + status[scrolling + max_choice -1 ], max_choice -1 , FALSE);scrollok (list, TRUE);wscrl (list,1 );scrollok (list, FALSE); } - scroll++; -print_item (list, items[(scroll + max_choice -1 ) *3 +1 ], - status[scroll + max_choice -1 ], max_choice -1 , TRUE); + scrolling++; +print_item (list, items[(scrolling + max_choice -1 ) *3 +1 ], + status[scrolling + max_choice -1 ], max_choice -1 , TRUE);print_arrows (dialog, choice, item_no, - scroll, box_y, box_x + check_x +5 , list_height); + scrolling, box_y, box_x + check_x +5 , list_height);wnoutrefresh (dialog);wrefresh (list);
@@ -267,12 +267,12 @@ int dialog_checklist(const char *title, }if (i != choice) {/* De-highlight current item */ -print_item (list, items[(scroll + choice) *3 +1 ], - status[scroll + choice], choice, FALSE); +print_item (list, items[(scrolling + choice) *3 +1 ], + status[scrolling + choice], choice, FALSE);/* Highlight new item */ choice = i; -print_item (list, items[(scroll + choice) *3 +1 ], - status[scroll + choice], choice, TRUE); +print_item (list, items[(scrolling + choice) *3 +1 ], + status[scrolling + choice], choice, TRUE);wnoutrefresh (dialog);wrefresh (list); }
@@ -282,7 +282,7 @@ int dialog_checklist(const char *title,case 'H' :case 'h' :case '?' : -fprintf (stderr,"%s" , items[(scroll + choice) *3 ]); +fprintf (stderr,"%s" , items[(scrolling + choice) *3 ]);delwin (dialog);free (status);return 1 ;
@@ -300,13 +300,13 @@ int dialog_checklist(const char *title,case ' ' :case '\n' :if (!button) { -if (!status[scroll + choice]) { +if (!status[scrolling + choice]) {for (i =0 ; i < item_no; i++) status[i] =0 ; - status[scroll + choice] =1 ; + status[scrolling + choice] =1 ;for (i =0 ; i < max_choice; i++) -print_item (list, items[(scroll + i) *3 +1 ], - status[scroll + i], i, i == choice); +print_item (list, items[(scrolling + i) *3 +1 ], + status[scrolling + i], i, i == choice); }wnoutrefresh (dialog);wrefresh (list);
@@ -315,7 +315,7 @@ int dialog_checklist(const char *title,if (status[i])fprintf (stderr,"%s" , items[i *3 ]); }else -fprintf (stderr,"%s" , items[(scroll + choice) *3 ]); +fprintf (stderr,"%s" , items[(scrolling + choice) *3 ]);delwin (dialog);free (status);return button;
scripts/kconfig/lxdialog/dialog.h +3 -3
@@ -146,14 +146,14 @@ void color_setup(void);void print_autowrap (WINDOW * win,const char *prompt,int width,int y,int x);void print_button (WINDOW * win,const char *label,int y,int x,int selected);void print_title (WINDOW *dialog,const char *title,int width); -void draw_box (WINDOW * win,int y,int x,int height,int width,chtype box, -chtype border); +void draw_box (WINDOW * win,int y,int x,int height,int width,chtype the_box, +chtype the_border);void draw_shadow (WINDOW * win,int y,int x,int height,int width);int first_alpha (const char *string,const char *exempt);int dialog_yesno (const char *title,const char *prompt,int height,int width);int dialog_msgbox (const char *title,const char *prompt,int height, -int width,int pause); +int width,int delay);int dialog_textbox (const char *title,const char *file,int height,int width); int dialog_menu(const char *title, const char *prompt, int height, int width, int menu_height, const char *choice, int item_no,
scripts/kconfig/lxdialog/inputbox.c +19 -19
@@ -45,8 +45,8 @@ int dialog_inputbox(const char *title, c const char *init) {int i, x, y, box_y, box_x, box_width; -int input_x =0 , scroll =0 , key =0 , button =-1 ; -char *instr = dialog_input_result; +int input_x =0 , scrolling =0 , key =0 , button =-1 ; +char *in_str = dialog_input_result;WINDOW *dialog;/* center dialog box on screen */
@@ -85,19 +85,19 @@ int dialog_inputbox(const char *title, cwattrset (dialog, inputbox_attr);if (!init) - instr[0 ] ='\0' ; + in_str[0 ] ='\0' ;else -strcpy (instr, init); +strcpy (in_str, init); - input_x =strlen (instr); + input_x =strlen (in_str);if (input_x >= box_width) { - scroll = input_x - box_width +1 ; + scrolling = input_x - box_width +1 ; input_x = box_width -1 ;for (i =0 ; i < box_width -1 ; i++) -waddch (dialog, instr[scroll + i]); +waddch (dialog, in_str[scrolling + i]); }else { -waddstr (dialog, instr); +waddstr (dialog, in_str); }wmove (dialog, box_y, box_x + input_x);
@@ -119,19 +119,19 @@ int dialog_inputbox(const char *title, ccontinue ;case KEY_BACKSPACE :case 127 : -if (input_x || scroll) { +if (input_x || scrolling) {wattrset (dialog, inputbox_attr);if (!input_x) { - scroll = scroll < box_width -1 ?0 : scroll - (box_width -1 ); + scrolling = scrolling < box_width -1 ?0 : scrolling - (box_width -1 );wmove (dialog, box_y, box_x);for (i =0 ; i < box_width; i++)waddch (dialog, - instr[scroll + input_x + i] ? - instr[scroll + input_x + i] :' ' ); - input_x =strlen (instr) - scroll; + in_str[scrolling + input_x + i] ? + in_str[scrolling + input_x + i] :' ' ); + input_x =strlen (in_str) - scrolling; }else input_x--; - instr[scroll + input_x] ='\0' ; + in_str[scrolling + input_x] ='\0' ;mvwaddch (dialog, box_y, input_x + box_x,' ' );wmove (dialog, box_y, input_x + box_x);wrefresh (dialog);
@@ -139,15 +139,15 @@ int dialog_inputbox(const char *title, ccontinue ;default :if (key <0x100 &&isprint (key)) { -if (scroll + input_x <MAX_LEN ) { +if (scrolling + input_x <MAX_LEN ) {wattrset (dialog, inputbox_attr); - instr[scroll + input_x] = key; - instr[scroll + input_x +1 ] ='\0' ; + in_str[scrolling + input_x] = key; + in_str[scrolling + input_x +1 ] ='\0' ;if (input_x == box_width -1 ) { - scroll++; + scrolling++;wmove (dialog, box_y, box_x);for (i =0 ; i < box_width -1 ; i++) -waddch (dialog, instr [scroll + i]); +waddch (dialog, in_str[scrolling + i]); }else {wmove (dialog, box_y, input_x++ + box_x);waddch (dialog, key);
scripts/kconfig/lxdialog/menubox.c +41 -41
196 lines not on this page · raw message
scripts/kconfig/lxdialog/msgbox.c +2 -2
@@ -26,7 +26,7 @@* if the parameter 'pause' is non-zero. */ int dialog_msgbox (const char *title,const char *prompt,int height,int width, -int pause) +int delay) {int i, x, y, key =0 ;WINDOW *dialog;
@@ -47,7 +47,7 @@ int dialog_msgbox(const char *title, conwattrset (dialog, dialog_attr);print_autowrap (dialog, prompt, width -2 ,1 ,2 ); -if (pause) { +if (delay) {wattrset (dialog, border_attr);mvwaddch (dialog, height -3 ,0 ,ACS_LTEE );for (i =0 ; i < width -2 ; i++)
scripts/kconfig/lxdialog/util.c +10 -10
@@ -288,7 +288,7 @@ void print_button(WINDOW * win, const ch*/ void draw_box (WINDOW * win,int y,int x,int height,int width, -chtype box,chtype border) +chtype the_box,chtype the_border) {int i, j;
@@ -297,23 +297,23 @@ draw_box(WINDOW * win, int y, int x, intwmove (win, y + i, x);for (j =0 ; j < width; j++)if (!i && !j) -waddch (win, border |ACS_ULCORNER ); +waddch (win, the_border |ACS_ULCORNER );else if (i == height -1 && !j) -waddch (win, border |ACS_LLCORNER ); +waddch (win, the_border |ACS_LLCORNER );else if (!i && j == width -1 ) -waddch (win, box |ACS_URCORNER ); +waddch (win, the_box |ACS_URCORNER );else if (i == height -1 && j == width -1 ) -waddch (win, box |ACS_LRCORNER ); +waddch (win, the_box |ACS_LRCORNER );else if (!i) -waddch (win, border |ACS_HLINE ); +waddch (win, the_border |ACS_HLINE );else if (i == height -1 ) -waddch (win, box |ACS_HLINE ); +waddch (win, the_box |ACS_HLINE );else if (!j) -waddch (win, border |ACS_VLINE ); +waddch (win, the_border |ACS_VLINE );else if (j == width -1 ) -waddch (win, box |ACS_VLINE ); +waddch (win, the_box |ACS_VLINE );else -waddch (win, box |' ' ); +waddch (win, the_box |' ' ); } }
PATCH 3/12 making the kernel -Wshadow clean - fix jiffies.h
In reply to Jesper JuhlFix -Wshadow warnings in include/linux/jiffies.h
---
include/linux/jiffies.h | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
include/linux/jiffies.h +4 -4
@@ -325,13 +325,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 jiffy,struct timespec *value) {/* * Convert jiffies to nanoseconds and separate with * one divide. */ -u64 nsec = (u64 )jiffies *TICK_NSEC ; +u64 nsec = (u64 )jiffy *TICK_NSEC ; value->tv_sec =div_long_long_rem (nsec,NSEC_PER_SEC , &value->tv_nsec); }
@@ -363,13 +363,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 jiffy,struct timeval *value) {/* * Convert jiffies to nanoseconds and separate with * one divide. */ -u64 nsec = (u64 )jiffies *TICK_NSEC ; +u64 nsec = (u64 )jiffy *TICK_NSEC ;long tv_usec; value->tv_sec =div_long_long_rem (nsec,NSEC_PER_SEC , &tv_usec);
PATCH 4/12 making the kernel -Wshadow clean - warnings related to 'up'
In reply to Jesper JuhlFix a few -Wshadow warnings related to variables of the name 'up' clashing with the global function name up() by renaming the variables.
---
drivers/char/vt_ioctl.c | 60 +++++++++++++++++++------------------- fs/select.c | 6 +-- kernel/user.c | 36 +++++++++++----------- 3 files changed, 51 insertions(+), 51 deletions(-)
drivers/char/vt_ioctl.c +30 -30
@@ -187,7 +187,7 @@ do_kdgkb_ioctl(int cmd, struct kbsentrystruct kbsentry *kbs;char *p;u_char *q; -u_char __user *up; +u_char __user *u;int sz;int delta;char *first_free, *fj, *fnw;
@@ -215,15 +215,15 @@ do_kdgkb_ioctl(int cmd, struct kbsentrycase KDGKBSENT : sz =sizeof (kbs->kb_string) -1 ;/* sz should have been a struct member */ - up = user_kdgkb->kb_string; + u = user_kdgkb->kb_string; p = func_table[i];if (p)for ( ; *p && sz; p++, sz--) -if (put_user (*p, up++)) { +if (put_user (*p, u++)) { ret = -EFAULT ; goto reterr; } -if (put_user ('\0' , up)) { +if (put_user ('\0' , u)) { ret = -EFAULT ; goto reterr; }
@@ -370,7 +370,7 @@ int vt_ioctl(struct tty_struct *tty, strstruct kbd_struct * kbd;unsigned int console;unsigned char ucval; -void __user *up = (void __user *)arg; +void __user *u = (void __user *)arg;int i, perm; console = vc->vc_num;
@@ -454,12 +454,12 @@ int vt_ioctl(struct tty_struct *tty, strif (!capable (CAP_SYS_TTY_CONFIG ))return -EPERM ; -if (copy_from_user (&kbrep, up,sizeof (struct kbd_repeat ))) +if (copy_from_user (&kbrep, u,sizeof (struct kbd_repeat )))return -EFAULT ; err =kbd_rate (&kbrep);if (err)return err; -if (copy_to_user (up, &kbrep,sizeof (struct kbd_repeat ))) +if (copy_to_user (u, &kbrep,sizeof (struct kbd_repeat )))return -EFAULT ;return 0 ; }
@@ -569,19 +569,19 @@ int vt_ioctl(struct tty_struct *tty, strcase KDSETKEYCODE :if (!capable (CAP_SYS_TTY_CONFIG )) perm=0 ; -return do_kbkeycode_ioctl (cmd, up, perm); +return do_kbkeycode_ioctl (cmd, u, perm);case KDGKBENT :case KDSKBENT : -return do_kdsk_ioctl (cmd, up, perm, kbd); +return do_kdsk_ioctl (cmd, u, perm, kbd);case KDGKBSENT :case KDSKBSENT : -return do_kdgkb_ioctl (cmd, up, perm); +return do_kdgkb_ioctl (cmd, u, perm);case KDGKBDIACR : { -struct kbdiacrs __user *a = up; +struct kbdiacrs __user *a = u;if (put_user (accent_table_size, &a->kb_cnt))return -EFAULT ;
@@ -592,7 +592,7 @@ int vt_ioctl(struct tty_struct *tty, strcase KDSKBDIACR : { -struct kbdiacrs __user *a = up; +struct kbdiacrs __user *a = u;unsigned int ct;if (!perm)
@@ -661,7 +661,7 @@ int vt_ioctl(struct tty_struct *tty, strif (!perm)return -EPERM ; -if (copy_from_user (&tmp, up,sizeof (struct vt_mode ))) +if (copy_from_user (&tmp, u,sizeof (struct vt_mode )))return -EFAULT ;if (tmp.mode !=VT_AUTO && tmp.mode !=VT_PROCESS )return -EINVAL ;
@@ -685,7 +685,7 @@ int vt_ioctl(struct tty_struct *tty, strmemcpy (&tmp, &vc->vt_mode,sizeof (struct vt_mode ));release_console_sem (); - rc =copy_to_user (up, &tmp,sizeof (struct vt_mode )); + rc =copy_to_user (u, &tmp,sizeof (struct vt_mode ));return rc ? -EFAULT :0 ; }
@@ -696,7 +696,7 @@ int vt_ioctl(struct tty_struct *tty, str*/ case VT_GETSTATE : { -struct vt_stat __user *vtstat = up; +struct vt_stat __user *vtstat = u;unsigned short state, mask;if (put_user (fg_console +1 , &vtstat->v_active))
@@ -840,7 +840,7 @@ int vt_ioctl(struct tty_struct *tty, strcase VT_RESIZE : { -struct vt_sizes __user *vtsizes = up; +struct vt_sizes __user *vtsizes = u;ushort ll,cc;if (!perm)return -EPERM ;
@@ -857,7 +857,7 @@ int vt_ioctl(struct tty_struct *tty, strcase VT_RESIZEX : { -struct vt_consize __user *vtconsize = up; +struct vt_consize __user *vtconsize = u;ushort ll,cc,vlin,clin,vcol,ccol;if (!perm)return -EPERM ;
@@ -911,7 +911,7 @@ int vt_ioctl(struct tty_struct *tty, str op.width =8 ; op.height =0 ; op.charcount =256 ; - op.data = up; + op.data = u;return con_font_op (vc_cons[fg_console].d, &op); }
@@ -921,21 +921,21 @@ int vt_ioctl(struct tty_struct *tty, str op.width =8 ; op.height =32 ; op.charcount =256 ; - op.data = up; + op.data = u;return con_font_op (vc_cons[fg_console].d, &op); }case PIO_CMAP :if (!perm)return -EPERM ; -return con_set_cmap (up); +return con_set_cmap (u);case GIO_CMAP : -return con_get_cmap (up); +return con_get_cmap (u);case PIO_FONTX :case GIO_FONTX : -return do_fontx_ioctl (cmd, up, perm, &op); +return do_fontx_ioctl (cmd, u, perm, &op);case PIO_FONTRESET : {
@@ -960,13 +960,13 @@ int vt_ioctl(struct tty_struct *tty, str }case KDFONTOP : { -if (copy_from_user (&op, up,sizeof (op))) +if (copy_from_user (&op, u,sizeof (op)))return -EFAULT ;if (!perm && op.op !=KD_FONT_OP_GET )return -EPERM ; i =con_font_op (vc, &op);if (i)return i; -if (copy_to_user (up, &op,sizeof (op))) +if (copy_to_user (u, &op,sizeof (op)))return -EFAULT ;return 0 ; }
@@ -974,24 +974,24 @@ int vt_ioctl(struct tty_struct *tty, strcase PIO_SCRNMAP :if (!perm)return -EPERM ; -return con_set_trans_old (up); +return con_set_trans_old (u);case GIO_SCRNMAP : -return con_get_trans_old (up); +return con_get_trans_old (u);case PIO_UNISCRNMAP :if (!perm)return -EPERM ; -return con_set_trans_new (up); +return con_set_trans_new (u);case GIO_UNISCRNMAP : -return con_get_trans_new (up); +return con_get_trans_new (u);case PIO_UNIMAPCLR : {struct unimapinit ui;if (!perm)return -EPERM ; - i =copy_from_user (&ui, up,sizeof (struct unimapinit )); + i =copy_from_user (&ui, u,sizeof (struct unimapinit ));if (i)return -EFAULT ;con_clear_unimap (vc, &ui);return 0 ;
@@ -999,7 +999,7 @@ int vt_ioctl(struct tty_struct *tty, strcase PIO_UNIMAP :case GIO_UNIMAP : -return do_unimap_ioctl (cmd, up, perm, vc); +return do_unimap_ioctl (cmd, u, perm, vc);case VT_LOCKSWITCH :if (!capable (CAP_SYS_TTY_CONFIG ))
fs/select.c +3 -3
@@ -524,17 +524,17 @@ asmlinkage long sys_pselect6(int n, fd_s fd_set __user *exp, struct timespec __user *tsp, void __user *sig) { size_t sigsetsize = 0; -sigset_t __user *up =NULL ; +sigset_t __user *u =NULL ;if (sig) {if (!access_ok (VERIFY_READ , sig,sizeof (void *)+sizeof (size_t )) - ||__get_user (up, (sigset_t __user *__user *)sig) + ||__get_user (u, (sigset_t __user *__user *)sig) ||__get_user (sigsetsize, (size_t __user *)(sig+sizeof (void *))))return -EFAULT ; } -return sys_pselect7 (n, inp, outp, exp, tsp, up, sigsetsize); +return sys_pselect7 (n, inp, outp, exp, tsp, u, sigsetsize); }#endif /* TIF_RESTORE_SIGMASK */
kernel/user.c +18 -18
@@ -56,14 +56,14 @@ struct user_struct root_user = {/* * These routines must be called with the uidhash spinlock held! */ -static inline void uid_hash_insert (struct user_struct *up,struct list_head *hashent) +static inline void uid_hash_insert (struct user_struct *u,struct list_head *hashent) { -list_add (&up->uidhash_list, hashent); +list_add (&u->uidhash_list, hashent); } -static inline void uid_hash_remove (struct user_struct *up) +static inline void uid_hash_remove (struct user_struct *u) { -list_del (&up->uidhash_list); +list_del (&u->uidhash_list); }static inline struct user_struct *uid_hash_find (uid_t uid,struct list_head *hashent)
@@ -101,20 +101,20 @@ struct user_struct *find_user(uid_t uid)return ret; } -void free_uid (struct user_struct *up) +void free_uid (struct user_struct *u) {unsigned long flags; -if (!up) +if (!u)return ;local_irq_save (flags); -if (atomic_dec_and_lock (&up->__count, &uidhash_lock)) { -uid_hash_remove (up); +if (atomic_dec_and_lock (&u->__count, &uidhash_lock)) { +uid_hash_remove (u);spin_unlock_irqrestore (&uidhash_lock, flags); -key_put (up->uid_keyring); -key_put (up->session_keyring); -kmem_cache_free (uid_cachep, up); +key_put (u->uid_keyring); +key_put (u->session_keyring); +kmem_cache_free (uid_cachep, u); }else {local_irq_restore (flags); }
@@ -123,13 +123,13 @@ void free_uid(struct user_struct *up)struct user_struct *alloc_uid (uid_t uid) {struct list_head *hashent =uidhashentry (uid); -struct user_struct *up; +struct user_struct *u;spin_lock_irq (&uidhash_lock); - up =uid_hash_find (uid, hashent); + u =uid_hash_find (uid, hashent);spin_unlock_irq (&uidhash_lock); -if (!up) { +if (!u) {struct user_struct *new; new =kmem_cache_alloc (uid_cachep,SLAB_KERNEL );
@@ -158,19 +158,19 @@ struct user_struct * alloc_uid(uid_t uid* on adding the same user already.. */ spin_lock_irq (&uidhash_lock); - up =uid_hash_find (uid, hashent); -if (up) { + u =uid_hash_find (uid, hashent); +if (u) {key_put (new->uid_keyring);key_put (new->session_keyring);kmem_cache_free (uid_cachep, new); }else {uid_hash_insert (new, hashent); - up = new; + u = new; }spin_unlock_irq (&uidhash_lock); } -return up; +return u; }void switch_uid (struct user_struct *new_user)
PATCH 5/12 making the kernel -Wshadow clean - warnings related to wbc and map_bh
In reply to Jesper JuhlFix -Wshadow warnings related to wbc and map_bh.
---
fs/buffer.c | 28 +++++++------- fs/direct-io.c | 24 ++++++------ fs/ext3/dir.c | 8 ++-- fs/mpage.c | 86 +++++++++++++++++++++---------------------- include/linux/fs.h | 2 - 5 files changed, 74 insertions(+), 74 deletions(-)
include/linux/fs.h +1 -1
@@ -352,7 +352,7 @@ struct address_space;struct writeback_control ;struct address_space_operations { -int (*writepage)(struct page *page,struct writeback_control *wbc); +int (*writepage)(struct page *page,struct writeback_control *wbctrl);int (*readpage)(struct file *,struct page *);void (*sync_page)(struct page *);
fs/mpage.c +43 -43
204 lines not on this page · raw message
fs/buffer.c +14 -14
@@ -2374,7 +2374,7 @@ int nobh_prepare_write(struct page *pagestruct inode *inode = page->mapping->host;const unsigned blkbits = inode->i_blkbits;const unsigned blocksize =1 << blkbits; -struct buffer_head map_bh; +struct buffer_head bh_map;struct buffer_head *read_bh[MAX_BUF_PER_PAGE ];unsigned block_in_page;unsigned block_start;
@@ -2390,7 +2390,7 @@ int nobh_prepare_write(struct page *pagereturn 0 ; block_in_file = (sector_t )page->index << (PAGE_CACHE_SHIFT - blkbits); - map_bh.b_page = page; + bh_map.b_page = page;/* * We loop across all blocks in the page, whether or not they are
@@ -2403,23 +2403,23 @@ int nobh_prepare_write(struct page *pageunsigned block_end = block_start + blocksize;int create; - map_bh.b_state =0 ; + bh_map.b_state =0 ; create =1 ;if (block_start >= to) create =0 ; - map_bh.b_size = blocksize; + bh_map.b_size = blocksize; ret =get_block (inode, block_in_file + block_in_page, - &map_bh, create); + &bh_map, create);if (ret) goto failed; -if (!buffer_mapped (&map_bh)) +if (!buffer_mapped (&bh_map)) is_mapped_to_disk =0 ; -if (buffer_new (&map_bh)) -unmap_underlying_metadata (map_bh.b_bdev, - map_bh.b_blocknr); +if (buffer_new (&bh_map)) +unmap_underlying_metadata (bh_map.b_bdev, + bh_map.b_blocknr);if (PageUptodate (page))continue ; -if (buffer_new (&map_bh) || !buffer_mapped (&map_bh)) { +if (buffer_new (&bh_map) || !buffer_mapped (&bh_map)) { kaddr =kmap_atomic (page,KM_USER0 );if (block_start < from) {memset (kaddr+block_start,0 , from-block_start);
@@ -2433,7 +2433,7 @@ int nobh_prepare_write(struct page *pagekunmap_atomic (kaddr,KM_USER0 );continue ; } -if (buffer_uptodate (&map_bh)) +if (buffer_uptodate (&bh_map))continue ;/* reiserfs does this */ if (block_start < from || block_end > to) {struct buffer_head *bh =alloc_buffer_head (GFP_NOFS );
@@ -2442,14 +2442,14 @@ int nobh_prepare_write(struct page *page ret = -ENOMEM ; goto failed; } - bh->b_state = map_bh.b_state; + bh->b_state = bh_map.b_state;atomic_set (&bh->b_count,0 ); bh->b_this_page =NULL ; bh->b_page = page; - bh->b_blocknr = map_bh.b_blocknr; + bh->b_blocknr = bh_map.b_blocknr; bh->b_size = blocksize; bh->b_data = (char *)(long )block_start; - bh->b_bdev = map_bh.b_bdev; + bh->b_bdev = bh_map.b_bdev; bh->b_private =NULL ; read_bh[nr_reads++] = bh; }
fs/direct-io.c +12 -12
@@ -512,7 +512,7 @@ static int dio_bio_reap(struct dio *dio)static int get_more_blocks (struct dio *dio) {int ret; -struct buffer_head *map_bh = &dio->map_bh; +struct buffer_head *bh_map = &dio->map_bh;sector_t fs_startblk;/* Into file, in filesystem-sized blocks */ unsigned long fs_count;/* Number of filesystem-sized blocks */ unsigned long dio_count;/* Number of dio_block-sized blocks */
@@ -533,8 +533,8 @@ static int get_more_blocks(struct dio *dif (dio_count & blkmask) fs_count++; - map_bh->b_state =0 ; - map_bh->b_size = fs_count << dio->inode->i_blkbits; + bh_map->b_state =0 ; + bh_map->b_size = fs_count << dio->inode->i_blkbits; create = dio->rw &WRITE ;if (dio->lock_type ==DIO_LOCKING ) {
@@ -552,7 +552,7 @@ static int get_more_blocks(struct dio *d* writes. */ ret = (*dio->get_block)(dio->inode, fs_startblk, - map_bh, create); + bh_map, create); }return ret; }
@@ -799,7 +799,7 @@ static int do_direct_IO(struct dio *dio)const unsigned blocks_per_page =PAGE_SIZE >> blkbits;struct page *page;unsigned block_in_page; -struct buffer_head *map_bh = &dio->map_bh; +struct buffer_head *bh_map = &dio->map_bh;int ret =0 ;/* The I/O can start at any block offset within the first page */
@@ -830,14 +830,14 @@ static int do_direct_IO(struct dio *dio)page_cache_release (page); goto out; } -if (!buffer_mapped (map_bh)) +if (!buffer_mapped (bh_map)) goto do_holes; dio->blocks_available = - map_bh->b_size >> dio->blkbits; + bh_map->b_size >> dio->blkbits; dio->next_block_for_io = - map_bh->b_blocknr << dio->blkfactor; -if (buffer_new (map_bh)) + bh_map->b_blocknr << dio->blkfactor; +if (buffer_new (bh_map))clean_blockdev_aliases (dio);if (!dio->blkfactor)
@@ -857,13 +857,13 @@ static int do_direct_IO(struct dio *dio)* the start of the fs block must be zeroed out * on-disk */ -if (!buffer_new (map_bh)) +if (!buffer_new (bh_map)) dio->next_block_for_io += dio_remainder; dio->blocks_available -= dio_remainder; } do_holes:/* Handle holes */ -if (!buffer_mapped (map_bh)) { +if (!buffer_mapped (bh_map)) {char *kaddr;loff_t i_size_aligned;
@@ -917,7 +917,7 @@ do_holes: this_chunk_bytes = this_chunk_blocks << blkbits;BUG_ON (this_chunk_bytes ==0 ); - dio->boundary =buffer_boundary (map_bh); + dio->boundary =buffer_boundary (bh_map); ret =submit_page_section (dio, page, offset_in_page, this_chunk_bytes, dio->next_block_for_io);if (ret) {
fs/ext3/dir.c +4 -4
@@ -127,17 +127,17 @@ static int ext3_readdir(struct file * fiwhile (!error && !stored && filp->f_pos < inode->i_size) {unsigned long blk = filp->f_pos >>EXT3_BLOCK_SIZE_BITS (sb); -struct buffer_head map_bh; +struct buffer_head bh_map;struct buffer_head *bh =NULL ; - map_bh.b_state =0 ; + bh_map.b_state =0 ; err =ext3_get_blocks_handle (NULL , inode, blk,1 , - &map_bh,0 ,0 ); + &bh_map,0 ,0 );if (err >0 ) {page_cache_readahead (sb->s_bdev->bd_inode->i_mapping, &filp->f_ra, filp, - map_bh.b_blocknr >> + bh_map.b_blocknr >> (PAGE_CACHE_SHIFT - inode->i_blkbits),1 ); bh =ext3_bread (NULL , inode, blk,0 , &err);
PATCH 6/12 making the kernel -Wshadow clean - fix checksum
In reply to Jesper JuhlSome -Wshadow fixes for include/[asm-i386|net]/checksum.h
include/asm/checksum.h:185: warning: declaration of 'sum' shadows a parameter include/asm/checksum.h:181: warning: shadowed declaration is here include/net/checksum.h:33: warning: declaration of 'sum' shadows a parameter include/net/checksum.h:31: warning: shadowed declaration is here
these show up several times each.
---
include/asm-i386/checksum.h | 4 ++-- include/net/checksum.h | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-)
include/asm-i386/checksum.h +2 -2
@@ -178,12 +178,12 @@ static __inline__ unsigned short int csu#define HAVE_CSUM_COPY_USER static __inline__unsigned int csum_and_copy_to_user (const unsigned char *src,unsigned char __user *dst, -int len,int sum, +int len,int csum,int *err_ptr) {might_sleep ();if (access_ok (VERIFY_WRITE , dst, len)) -return csum_partial_copy_generic (src, (__force unsigned char *)dst, len, sum,NULL , err_ptr); +return csum_partial_copy_generic (src, (__force unsigned char *)dst, len, csum,NULL , err_ptr);if (len) *err_ptr = -EFAULT ;
include/net/checksum.h +6 -6
@@ -28,27 +28,27 @@#ifndef _HAVE_ARCH_COPY_AND_CSUM_FROM_USERstatic inline unsigned int csum_and_copy_from_user (const unsigned char __user *src,unsigned char *dst, -int len,int sum,int *err_ptr) +int len,int csum,int *err_ptr) {if (access_ok (VERIFY_READ , src, len)) -return csum_partial_copy_from_user (src, dst, len, sum, err_ptr); +return csum_partial_copy_from_user (src, dst, len, csum, err_ptr);if (len) *err_ptr = -EFAULT ; -return sum; +return csum; }#endif #ifndef HAVE_CSUM_COPY_USER static __inline__unsigned int csum_and_copy_to_user -(const unsigned char *src,unsigned char __user *dst,int len,unsigned int sum,int *err_ptr) +(const unsigned char *src,unsigned char __user *dst,int len,unsigned int csum,int *err_ptr) { - sum =csum_partial (src, len, sum); + sum =csum_partial (src, len, csum);if (access_ok (VERIFY_WRITE , dst, len)) {if (copy_to_user (dst, src, len) ==0 ) -return sum; +return csum; }if (len) *err_ptr = -EFAULT ;
PATCH 7/12 making the kernel -Wshadow clean - fix vgacon
In reply to Jesper JuhlFix a few -Wshadow warnings in drivers/video/console/vgacon.c
---
drivers/video/console/vgacon.c | 72 +++++++++++++++---------------- 1 files changed, 35 insertions(+), 37 deletions(-)
drivers/video/console/vgacon.c +35 -37
@@ -855,14 +855,14 @@ static struct {unsigned char ClockingMode;/* Seq-Controller:01h */ } vga_state; -static void vga_vesa_blank (struct vgastate *state,int mode) +static void vga_vesa_blank (struct vgastate *condition,int mode) {/* save original values of VGA controller registers */ if (!vga_vesa_blanked) {spin_lock_irq (&vga_lock); - vga_state.SeqCtrlIndex =vga_r (state->vgabase,VGA_SEQ_I ); + vga_state.SeqCtrlIndex =vga_r (condition->vgabase,VGA_SEQ_I ); vga_state.CrtCtrlIndex =inb_p (vga_video_port_reg); - vga_state.CrtMiscIO =vga_r (state->vgabase,VGA_MIS_R ); + vga_state.CrtMiscIO =vga_r (condition->vgabase,VGA_MIS_R );spin_unlock_irq (&vga_lock);outb_p (0x00 , vga_video_port_reg);/* HorizontalTotal */
@@ -881,17 +881,17 @@ static void vga_vesa_blank(struct vgasta vga_state.EndVertRetrace =inb_p (vga_video_port_val);outb_p (0x17 , vga_video_port_reg);/* ModeControl */ vga_state.ModeControl =inb_p (vga_video_port_val); - vga_state.ClockingMode =vga_rseq (state->vgabase,VGA_SEQ_CLOCK_MODE ); + vga_state.ClockingMode =vga_rseq (condition->vgabase,VGA_SEQ_CLOCK_MODE ); }/* assure that video is enabled */ /* "0x20" is VIDEO_ENABLE_bit in register 01 of sequencer */ spin_lock_irq (&vga_lock); -vga_wseq (state->vgabase,VGA_SEQ_CLOCK_MODE , vga_state.ClockingMode |0x20 ); +vga_wseq (condition->vgabase,VGA_SEQ_CLOCK_MODE , vga_state.ClockingMode |0x20 );/* test for vertical retrace in process.... */ if ((vga_state.CrtMiscIO &0x80 ) ==0x80 ) -vga_w (state->vgabase,VGA_MIS_W , vga_state.CrtMiscIO &0xEF ); +vga_w (condition->vgabase,VGA_MIS_W , vga_state.CrtMiscIO &0xEF );/* * Set <End of vertical retrace> to minimum (0) and
@@ -920,16 +920,16 @@ static void vga_vesa_blank(struct vgasta }/* restore both index registers */ -vga_w (state->vgabase,VGA_SEQ_I , vga_state.SeqCtrlIndex); +vga_w (condition->vgabase,VGA_SEQ_I , vga_state.SeqCtrlIndex);outb_p (vga_state.CrtCtrlIndex, vga_video_port_reg);spin_unlock_irq (&vga_lock); } -static void vga_vesa_unblank (struct vgastate *state) +static void vga_vesa_unblank (struct vgastate *condition) {/* restore original values of VGA controller registers */ spin_lock_irq (&vga_lock); -vga_w (state->vgabase,VGA_MIS_W , vga_state.CrtMiscIO); +vga_w (condition->vgabase,VGA_MIS_W , vga_state.CrtMiscIO);outb_p (0x00 , vga_video_port_reg);/* HorizontalTotal */ outb_p (vga_state.HorizontalTotal, vga_video_port_val);
@@ -948,24 +948,24 @@ static void vga_vesa_unblank(struct vgasoutb_p (0x17 , vga_video_port_reg);/* ModeControl */ outb_p (vga_state.ModeControl, vga_video_port_val);/* ClockingMode */ -vga_wseq (state->vgabase,VGA_SEQ_CLOCK_MODE , vga_state.ClockingMode); +vga_wseq (condition->vgabase,VGA_SEQ_CLOCK_MODE , vga_state.ClockingMode);/* restore index/control registers */ -vga_w (state->vgabase,VGA_SEQ_I , vga_state.SeqCtrlIndex); +vga_w (condition->vgabase,VGA_SEQ_I , vga_state.SeqCtrlIndex);outb_p (vga_state.CrtCtrlIndex, vga_video_port_reg);spin_unlock_irq (&vga_lock); } -static void vga_pal_blank (struct vgastate *state) +static void vga_pal_blank (struct vgastate *condition) {int i; -vga_w (state->vgabase,VGA_PEL_MSK ,0xff ); +vga_w (condition->vgabase,VGA_PEL_MSK ,0xff );for (i =0 ; i <16 ; i++) { -vga_w (state->vgabase,VGA_PEL_IW , i); -vga_w (state->vgabase,VGA_PEL_D ,0 ); -vga_w (state->vgabase,VGA_PEL_D ,0 ); -vga_w (state->vgabase,VGA_PEL_D ,0 ); +vga_w (condition->vgabase,VGA_PEL_IW , i); +vga_w (condition->vgabase,VGA_PEL_D ,0 ); +vga_w (condition->vgabase,VGA_PEL_D ,0 ); +vga_w (condition->vgabase,VGA_PEL_D ,0 ); } }
@@ -1027,7 +1027,7 @@ static int vgacon_blank(struct vc_data *#define blackwmap 0xa0000#define cmapsz 8192 -static int vgacon_do_font_op (struct vgastate *state,char *arg,int set,int ch512) +static int vgacon_do_font_op (struct vgastate *condition,char *arg,int set,int ch512) {unsigned short video_port_status = vga_video_port_reg +6 ;int font_select =0x00 , beg, i;
@@ -1075,20 +1075,20 @@ static int vgacon_do_font_op(struct vgasunlock_kernel ();spin_lock_irq (&vga_lock);/* First, the Sequencer */ -vga_wseq (state->vgabase,VGA_SEQ_RESET ,0x1 ); +vga_wseq (condition->vgabase,VGA_SEQ_RESET ,0x1 );/* CPU writes only to map 2 */ -vga_wseq (state->vgabase,VGA_SEQ_PLANE_WRITE ,0x04 ); +vga_wseq (condition->vgabase,VGA_SEQ_PLANE_WRITE ,0x04 );/* Sequential addressing */ -vga_wseq (state->vgabase,VGA_SEQ_MEMORY_MODE ,0x07 ); +vga_wseq (condition->vgabase,VGA_SEQ_MEMORY_MODE ,0x07 );/* Clear synchronous reset */ -vga_wseq (state->vgabase,VGA_SEQ_RESET ,0x03 ); +vga_wseq (condition->vgabase,VGA_SEQ_RESET ,0x03 );/* Now, the graphics controller, select map 2 */ -vga_wgfx (state->vgabase,VGA_GFX_PLANE_READ ,0x02 ); +vga_wgfx (condition->vgabase,VGA_GFX_PLANE_READ ,0x02 );/* disable odd-even addressing */ -vga_wgfx (state->vgabase,VGA_GFX_MODE ,0x00 ); +vga_wgfx (condition->vgabase,VGA_GFX_MODE ,0x00 );/* map start at A000:0000 */ -vga_wgfx (state->vgabase,VGA_GFX_MISC ,0x00 ); +vga_wgfx (condition->vgabase,VGA_GFX_MISC ,0x00 );spin_unlock_irq (&vga_lock);if (arg) {
@@ -1118,28 +1118,26 @@ static int vgacon_do_font_op(struct vgasspin_lock_irq (&vga_lock);/* First, the sequencer, Synchronous reset */ -vga_wseq (state->vgabase,VGA_SEQ_RESET ,0x01 ); +vga_wseq (condition->vgabase,VGA_SEQ_RESET ,0x01 );/* CPU writes to maps 0 and 1 */ -vga_wseq (state->vgabase,VGA_SEQ_PLANE_WRITE ,0x03 ); +vga_wseq (condition->vgabase,VGA_SEQ_PLANE_WRITE ,0x03 );/* odd-even addressing */ -vga_wseq (state->vgabase,VGA_SEQ_MEMORY_MODE ,0x03 ); +vga_wseq (condition->vgabase,VGA_SEQ_MEMORY_MODE ,0x03 );/* Character Map Select */ if (set) -vga_wseq (state->vgabase,VGA_SEQ_CHARACTER_MAP , font_select); +vga_wseq (condition->vgabase,VGA_SEQ_CHARACTER_MAP , font_select);/* clear synchronous reset */ -vga_wseq (state->vgabase,VGA_SEQ_RESET ,0x03 ); +vga_wseq (condition->vgabase,VGA_SEQ_RESET ,0x03 );/* Now, the graphics controller, select map 0 for CPU */ -vga_wgfx (state->vgabase,VGA_GFX_PLANE_READ ,0x00 ); +vga_wgfx (condition->vgabase,VGA_GFX_PLANE_READ ,0x00 );/* enable even-odd addressing */ -vga_wgfx (state->vgabase,VGA_GFX_MODE ,0x10 ); +vga_wgfx (condition->vgabase,VGA_GFX_MODE ,0x10 );/* map starts at b800:0 or b000:0 */ -vga_wgfx (state->vgabase,VGA_GFX_MISC , beg); +vga_wgfx (condition->vgabase,VGA_GFX_MISC , beg);/* if 512 char mode is already enabled don't re-enable it. */ if ((set) && (ch512 != vga_512_chars)) { -int i; -/* attribute controller */ for (i =0 ; i <MAX_NR_CONSOLES ; i++) {struct vc_data *c = vc_cons[i].d;
@@ -1151,11 +1149,11 @@ static int vgacon_do_font_op(struct vgas512-char: disable intensity bit */ inb_p (video_port_status);/* clear address flip-flop */ /* color plane enable register */ -vga_wattr (state->vgabase,VGA_ATC_PLANE_ENABLE , ch512 ?0x07 :0x0f ); +vga_wattr (condition->vgabase,VGA_ATC_PLANE_ENABLE , ch512 ?0x07 :0x0f );/* Wilton (1987) mentions the following; I don't know what it means, but it works, and it appears necessary */ inb_p (video_port_status); -vga_wattr (state->vgabase,VGA_AR_ENABLE_DISPLAY ,0 ); +vga_wattr (condition->vgabase,VGA_AR_ENABLE_DISPLAY ,0 ); }spin_unlock_irq (&vga_lock);lock_kernel ();
PATCH 8/12 making the kernel -Wshadow clean - fix keyboard.c
In reply to Jesper JuhlFix -Wshadow warnings in drivers/char/keyboard.c
---
drivers/char/keyboard.c | 58 +++++++++++++++++++------------------- 1 files changed, 29 insertions(+), 29 deletions(-)
drivers/char/keyboard.c +29 -29
@@ -266,7 +266,7 @@ void kd_mksound(unsigned int hz, unsigne* Setting the keyboard rate. */ -int kbd_rate (struct kbd_repeat *rep) +int kbd_rate (struct kbd_repeat *repeat) {struct list_head *node;unsigned int d =0 ;
@@ -277,16 +277,16 @@ int kbd_rate(struct kbd_repeat *rep)struct input_dev *dev = handle->dev;if (test_bit (EV_REP , dev->evbit)) { -if (rep->delay >0 ) -input_event (dev,EV_REP ,REP_DELAY , rep->delay); -if (rep->period >0 ) -input_event (dev,EV_REP ,REP_PERIOD , rep->period); +if (repeat->delay >0 ) +input_event (dev,EV_REP ,REP_DELAY , repeat->delay); +if (repeat->period >0 ) +input_event (dev,EV_REP ,REP_PERIOD , repeat->period); d = dev->rep[REP_DELAY ]; p = dev->rep[REP_PERIOD ]; } } - rep->delay = d; - rep->period = p; + repeat->delay = d; + repeat->period = p;return 0 ; }
@@ -944,28 +944,28 @@ unsigned char getledstate(void)return ledstate; } -void setledstate (struct kbd_struct *kbd,unsigned int led) +void setledstate (struct kbd_struct *kbrd,unsigned int led) {if (!(led & ~7 )) { ledioctl = led; - kbd->ledmode =LED_SHOW_IOCTL ; + kbrd->ledmode =LED_SHOW_IOCTL ; }else - kbd->ledmode =LED_SHOW_FLAGS ; + kbrd->ledmode =LED_SHOW_FLAGS ;set_leds (); }static inline unsigned char getleds (void ) { -struct kbd_struct *kbd = kbd_table + fg_console; +struct kbd_struct *kbrd = kbd_table + fg_console;unsigned char leds;int i; -if (kbd->ledmode ==LED_SHOW_IOCTL ) +if (kbrd->ledmode ==LED_SHOW_IOCTL )return ledioctl; - leds = kbd->ledflagstate; + leds = kbrd->ledflagstate; -if (kbd->ledmode ==LED_SHOW_MEM ) { +if (kbrd->ledmode ==LED_SHOW_MEM ) {for (i =0 ; i <3 ; i++)if (ledptrs[i].valid) {if (*ledptrs[i].addr & ledptrs[i].mask)
@@ -1126,7 +1126,7 @@ static void kbd_rawcode(unsigned char daput_queue (vc, data); } -static void kbd_keycode (unsigned int keycode,int down, +static void kbd_keycode (unsigned int keycode,int down_key,int hw_raw,struct pt_regs *regs) {struct vc_data *vc = vc_cons[fg_console].d;
@@ -1145,35 +1145,35 @@ static void kbd_keycode(unsigned int key kbd = kbd_table + fg_console;if (keycode ==KEY_LEFTALT || keycode ==KEY_RIGHTALT ) - sysrq_alt = down ? keycode :0 ; + sysrq_alt = down_key ? keycode :0 ;#ifdef CONFIG_SPARC if (keycode ==KEY_STOP ) - sparc_l1_a_state = down; + sparc_l1_a_state = down_key;#endif - rep = (down ==2 ); + rep = (down_key ==2 );#ifdef CONFIG_MAC_EMUMOUSEBTN -if (mac_hid_mouse_emulate_buttons (1 , keycode, down)) +if (mac_hid_mouse_emulate_buttons (1 , keycode, down_key))return ;#endif /* CONFIG_MAC_EMUMOUSEBTN */ if ((raw_mode = (kbd->kbdmode ==VC_RAW )) && !hw_raw) -if (emulate_raw (vc, keycode, !down <<7 )) +if (emulate_raw (vc, keycode, !down_key <<7 ))if (keycode <BTN_MISC )printk (KERN_WARNING "keyboard.c: can't emulate rawmode for keycode %d\n" , keycode);#ifdef CONFIG_MAGIC_SYSRQ /* Handle the SysRq Hack */ -if (keycode ==KEY_SYSRQ && (sysrq_down || (down ==1 && sysrq_alt))) { +if (keycode ==KEY_SYSRQ && (sysrq_down || (down_key ==1 && sysrq_alt))) {if (!sysrq_down) { - sysrq_down = down; + sysrq_down = down_key; sysrq_alt_use = sysrq_alt; }return ; } -if (sysrq_down && !down && keycode == sysrq_alt_use) +if (sysrq_down && !down_key && keycode == sysrq_alt_use) sysrq_down =0 ; -if (sysrq_down && down && !rep) { +if (sysrq_down && down_key && !rep) {handle_sysrq (kbd_sysrq_xlate[keycode], regs, tty);return ; }
@@ -1196,16 +1196,16 @@ static void kbd_keycode(unsigned int key* which should be enough. */ if (keycode <128 ) { -put_queue (vc, keycode | (!down <<7 )); +put_queue (vc, keycode | (!down_key <<7 )); }else { -put_queue (vc, !down <<7 ); +put_queue (vc, !down_key <<7 );put_queue (vc, (keycode >>7 ) |0x80 );put_queue (vc, keycode |0x80 ); } raw_mode =1 ; } -if (down) +if (down_key)set_bit (keycode, key_down);else clear_bit (keycode, key_down);
@@ -1241,7 +1241,7 @@ static void kbd_keycode(unsigned int key type =KTYP (keysym);if (type <0xf0 ) { -if (down && !raw_mode) +if (down_key && !raw_mode)to_utf8 (vc, keysym);return ; }
@@ -1260,7 +1260,7 @@ static void kbd_keycode(unsigned int key } } - (*k_handler[type])(vc, keysym &0xff , !down, regs); + (*k_handler[type])(vc, keysym &0xff , !down_key, regs);if (type !=KT_SLOCK ) kbd->slockstate =0 ;
PATCH 9/12 making the kernel -Wshadow clean - neighbour.h and 'proc_handler'
In reply to Jesper JuhlFix several hundred -Wshadow warnings, caused by include/net/neighbour.h, by renaming the 'proc_handler' argument in the neigh_sysctl_register() prototype to 'handler', just like it is named in net/core/neighbour.c
---
include/net/neighbour.h | 2 +- 1 files changed, 1 insertion(+), 1 deletion(-)
include/net/neighbour.h +1 -1
@@ -276,7 +276,7 @@ extern int neigh_sysctl_register(strucstruct neigh_parms *p, int p_id, int pdev_id, char *p_name, - proc_handler *proc_handler, + proc_handler *handler, ctl_handler *strategy);extern void neigh_sysctl_unregister (struct neigh_parms *p);
PATCH 10/12 making the kernel -Wshadow clean - mm/truncate.c
In reply to Jesper JuhlFix -Wshadow warnings in mm/truncate.c
---
mm/truncate.c | 24 ++++++++++++------------ 1 files changed, 12 insertions(+), 12 deletions(-)
mm/truncate.c +12 -12
@@ -127,15 +127,15 @@ void truncate_inode_pages_range(struct a pagevec_lookup(&pvec, mapping, next, PAGEVEC_SIZE)) {for (i =0 ; i <pagevec_count (&pvec); i++) {struct page *page = pvec.pages[i]; -pgoff_t page_index = page->index; +pgoff_t page_idx = page->index; -if (page_index > end) { - next = page_index; +if (page_idx > end) { + next = page_idx;break ; } -if (page_index > next) - next = page_index; +if (page_idx > next) + next = page_idx; next++;if (TestSetPageLocked (page))continue ;
@@ -298,7 +298,7 @@ int invalidate_inode_pages2_range(struct min(end - next, (pgoff_t)PAGEVEC_SIZE - 1) + 1)) {for (i =0 ; !ret && i <pagevec_count (&pvec); i++) {struct page *page = pvec.pages[i]; -pgoff_t page_index; +pgoff_t page_idx;int was_dirty;lock_page (page);
@@ -306,11 +306,11 @@ int invalidate_inode_pages2_range(structunlock_page (page);continue ; } - page_index = page->index; - next = page_index +1 ; + page_idx = page->index; + next = page_idx +1 ;if (next ==0 ) wrapped =1 ; -if (page_index > end) { +if (page_idx > end) {unlock_page (page);break ; }
@@ -321,8 +321,8 @@ int invalidate_inode_pages2_range(struct* Zap the rest of the file in one hit. */ unmap_mapping_range (mapping, - (loff_t )page_index<<PAGE_CACHE_SHIFT , - (loff_t)(end - page_index +1 ) + (loff_t )page_idx<<PAGE_CACHE_SHIFT , + (loff_t)(end - page_idx +1 ) <<PAGE_CACHE_SHIFT ,0 ); did_range_unmap =1 ;
@@ -331,7 +331,7 @@ int invalidate_inode_pages2_range(struct* Just zap this page */ unmap_mapping_range (mapping, - (loff_t )page_index<<PAGE_CACHE_SHIFT , + (loff_t )page_idx<<PAGE_CACHE_SHIFT ,PAGE_CACHE_SIZE ,0 ); } }
PATCH 11/12 making the kernel -Wshadow clean - USB & completion
In reply to Jesper Juhlinclude/linux/usb.h causes a lot of -Wshadow warnings - fix them.
include/linux/usb.h:901: warning: declaration of 'complete' shadows a global declaration include/linux/completion.h:52: warning: shadowed declaration is here include/linux/usb.h:932: warning: declaration of 'complete' shadows a global declaration include/linux/completion.h:52: warning: shadowed declaration is here include/linux/usb.h:967: warning: declaration of 'complete' shadows a global declaration include/linux/completion.h:52: warning: shadowed declaration is here
---
include/linux/usb.h | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-)
include/linux/usb.h +9 -9
@@ -886,7 +886,7 @@ struct urb* @setup_packet: pointer to the setup_packet buffer * @transfer_buffer: pointer to the transfer buffer * @buffer_length: length of the transfer buffer -* @complete: pointer to the usb_complete_t function +* @complete_fn: pointer to the usb_complete_t function * @context: what to set the urb context to. * * Initializes a control urb with the proper information needed to submit
@@ -898,7 +898,7 @@ static inline void usb_fill_control_urbunsigned char *setup_packet, void *transfer_buffer, int buffer_length, - usb_complete_t complete, + usb_complete_t complete_fn, void *context) { spin_lock_init(&urb->lock);
@@ -907,7 +907,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 = complete_fn; urb->context = context; }
@@ -918,7 +918,7 @@ static inline void usb_fill_control_urb* @pipe: the endpoint pipe * @transfer_buffer: pointer to the transfer buffer * @buffer_length: length of the transfer buffer -* @complete: pointer to the usb_complete_t function +* @complete_fn: pointer to the usb_complete_t function * @context: what to set the urb context to. * * Initializes a bulk urb with the proper information needed to submit it
@@ -929,7 +929,7 @@ static inline void usb_fill_bulk_urb (stunsigned int pipe, void *transfer_buffer, int buffer_length, - usb_complete_t complete, + usb_complete_t complete_fn, void *context) { spin_lock_init(&urb->lock);
@@ -937,7 +937,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 = complete_fn; urb->context = context; }
@@ -948,7 +948,7 @@ static inline void usb_fill_bulk_urb (st* @pipe: the endpoint pipe * @transfer_buffer: pointer to the transfer buffer * @buffer_length: length of the transfer buffer -* @complete: pointer to the usb_complete_t function +* @complete_fn: pointer to the usb_complete_t function * @context: what to set the urb context to. * @interval: what to set the urb interval to, encoded like * the endpoint descriptor's bInterval value.
@@ -964,7 +964,7 @@ static inline void usb_fill_int_urb (strunsigned int pipe, void *transfer_buffer, int buffer_length, - usb_complete_t complete, + usb_complete_t complete_fn, void *context, int interval) {
@@ -973,7 +973,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 = complete_fn; urb->context = context;if (dev->speed ==USB_SPEED_HIGH ) urb->interval =1 << (interval -1 );
PATCH 12/12 making the kernel -Wshadow clean - 'irq' shadows local and global
In reply to Jesper JuhlGet rid of various -Wshadow warnings related to 'irq' shadowing a local or global.
---
arch/i386/kernel/acpi/boot.c | 2 drivers/cdrom/mcdx.c | 7 - drivers/char/cyclades.c | 18 +-- drivers/char/esp.c | 4 drivers/char/synclink.c | 10 +- drivers/char/watchdog/eurotechwdt.c | 2 drivers/char/watchdog/wdt.c | 2 drivers/char/watchdog/wdt_pci.c | 2 drivers/isdn/pcbit/drv.c | 10 +- drivers/isdn/pcbit/module.c | 2 drivers/mmc/wbsd.c | 32 +++--- drivers/net/seeq8005.c | 2 include/linux/interrupt.h | 28 ++--- include/linux/irq.h | 128 +++++++++++++------------- include/linux/random.h | 4 sound/isa/gus/interwave.c | 3 16 files changed, 129 insertions(+), 127 deletions(-)
arch/i386/kernel/acpi/boot.c +1 -1
@@ -482,7 +482,7 @@ int acpi_register_gsi(u32 gsi, int trigg* Make sure all (legacy) PCI IRQs are set as level-triggered. */ if (acpi_irq_model ==ACPI_IRQ_MODEL_PIC ) { -extern void eisa_set_level_irq (unsigned int irq); +extern void eisa_set_level_irq (unsigned int );if (triggering ==ACPI_LEVEL_SENSITIVE )eisa_set_level_irq (gsi);
drivers/cdrom/mcdx.c +4 -3
@@ -265,7 +265,7 @@ static unsigned int msf2log(const structstatic unsigned int uint2bcd (unsigned int );static unsigned int bcd2uint (unsigned char );static unsigned port (int *); -static int irq (int *); +static int get_irq (int *);static void mcdx_delay (struct s_drive_stuff *,long jifs);static int mcdx_transfer (struct s_drive_stuff *,char *buf,int sector,int nr_sectors);
@@ -1104,7 +1104,7 @@ static int __init mcdx_init_drive(int dr stuffp->toc =NULL ;/* this should be NULL already */ /* setup our irq and i/o addresses */ - stuffp->irq =irq (mcdx_drive_map[drive]); + stuffp->irq =get_irq (mcdx_drive_map[drive]); stuffp->wreg_data = stuffp->rreg_data =port (mcdx_drive_map[drive]); stuffp->wreg_reset = stuffp->rreg_status = stuffp->wreg_data +1 ; stuffp->wreg_hcon = stuffp->wreg_reset +1 ;
@@ -1492,7 +1492,8 @@ static unsigned port(int *ip) {return ip[0 ]; } -static int irq (int *ip) + +static int get_irq (int *ip) {return ip[1 ]; }
drivers/char/cyclades.c +9 -9
@@ -1017,13 +1017,13 @@ cyy_issue_cmd(void __iomem *base_addr, ustatic unsigned detect_isa_irq (void __iomem *address) { -int irq; +int intr;unsigned long irqs, flags;int save_xir, save_car;int index =0 ;/* IRQ probing is only for ISA */ /* forget possible initially masked and pending IRQ */ - irq =probe_irq_off (probe_irq_on ()); + intr =probe_irq_off (probe_irq_on ());/* Clear interrupts on the board first */ cy_writeb (address + (Cy_ClrIntr<<index),0 );
@@ -1047,7 +1047,7 @@ detect_isa_irq(void __iomem *address)udelay (5000L );/* Check which interrupt is in use */ - irq =probe_irq_off (irqs); + intr =probe_irq_off (irqs);/* Clean up */ save_xir = (u_char )cy_readb (address + (CyTIR<<index));
@@ -1060,7 +1060,7 @@ detect_isa_irq(void __iomem *address)cy_writeb (address + (Cy_ClrIntr<<index),0 );/* Cy_ClrIntr is 0x1800 */ - return (irq > 0)? irq : 0; + return (intr > 0) ? intr : 0; }#endif /* CONFIG_ISA */
@@ -1069,7 +1069,7 @@ detect_isa_irq(void __iomem *address)received, out buffer empty, modem change, etc. */ static irqreturn_t -cyy_interrupt (int irq,void *dev_id,struct pt_regs *regs) +cyy_interrupt (int intr,void *dev_id,struct pt_regs *regs) {struct tty_struct *tty;int status;
@@ -1089,7 +1089,7 @@ cyy_interrupt(int irq, void *dev_id, strint len;if ((cinfo = (struct cyclades_card *)dev_id) ==0 ){#ifdef CY_DEBUG_INTERRUPTS -printk ("cyy_interrupt: spurious interrupt %d\n\r" , irq); +printk ("cyy_interrupt: spurious interrupt %d\n\r" , intr);#endif return IRQ_NONE ;/* spurious interrupt */ }
@@ -1814,20 +1814,20 @@ cyz_handle_cmd(struct cyclades_card *cin#ifdef CONFIG_CYZ_INTR static irqreturn_t -cyz_interrupt (int irq,void *dev_id,struct pt_regs *regs) +cyz_interrupt (int intr,void *dev_id,struct pt_regs *regs) {struct cyclades_card *cinfo;if ((cinfo = (struct cyclades_card *)dev_id) ==0 ){#ifdef CY_DEBUG_INTERRUPTS -printk ("cyz_interrupt: spurious interrupt %d\n\r" , irq); +printk ("cyz_interrupt: spurious interrupt %d\n\r" , intr);#endif return IRQ_NONE ;/* spurious interrupt */ }if (!ISZLOADED (*cinfo)) {#ifdef CY_DEBUG_INTERRUPTS -printk ("cyz_interrupt: board not yet loaded (IRQ%d).\n\r" , irq); +printk ("cyz_interrupt: board not yet loaded (IRQ%d).\n\r" , intr);#endif return IRQ_NONE ; }
drivers/char/esp.c +2 -2
@@ -615,7 +615,7 @@ static inline void check_modem_status(st/* * This is the serial driver's interrupt routine */ -static irqreturn_t rs_interrupt_single (int irq,void *dev_id, +static irqreturn_t rs_interrupt_single (int intr,void *dev_id,struct pt_regs *regs) {struct esp_struct * info;
@@ -623,7 +623,7 @@ static irqreturn_t rs_interrupt_single(iunsigned int scratch;#ifdef SERIAL_DEBUG_INTR -printk ("rs_interrupt_single(%d)..." , irq); +printk ("rs_interrupt_single(%d)..." , intr);#endif info = (struct esp_struct *)dev_id; err_status =0 ;
drivers/char/synclink.c +5 -5
@@ -1698,13 +1698,13 @@ static void mgsl_isr_transmit_dma( struc* * Arguments: * -* irq interrupt number that caused interrupt +* intr interrupt number that caused interrupt * dev_id device ID supplied during interrupt registration * regs interrupted processor context * * Return Value: None */ -static irqreturn_t mgsl_interrupt (int irq,void *dev_id,struct pt_regs * regs) +static irqreturn_t mgsl_interrupt (int intr,void *dev_id,struct pt_regs *regs) {struct mgsl_struct * info;u16 UscVector;
@@ -1712,7 +1712,7 @@ static irqreturn_t mgsl_interrupt(int irif ( debug_level >=DEBUG_LEVEL_ISR )printk ("%s(%d):mgsl_interrupt(%d)entry.\n" , - __FILE__,__LINE__,irq); + __FILE__,__LINE__,intr); info = (struct mgsl_struct *)dev_id;if (!info)
@@ -1742,7 +1742,7 @@ static irqreturn_t mgsl_interrupt(int irif ( info->isr_overflow ) {printk (KERN_ERR "%s(%d):%s isr overflow irq=%d\n" , - __FILE__,__LINE__,info->device_name, irq); + __FILE__,__LINE__,info->device_name, intr);usc_DisableMasterIrqBit (info);usc_DisableDmaInterrupts (info,DICR_MASTER );break ;
@@ -1765,7 +1765,7 @@ static irqreturn_t mgsl_interrupt(int irif ( debug_level >=DEBUG_LEVEL_ISR )printk ("%s(%d):mgsl_interrupt(%d)exit.\n" , - __FILE__,__LINE__,irq); + __FILE__,__LINE__,intr);return IRQ_HANDLED ; }/* end of mgsl_interrupt() */
drivers/char/watchdog/eurotechwdt.c +1 -1
@@ -153,7 +153,7 @@ static void eurwdt_activate_timer(void)* Kernel methods. */ -static irqreturn_t eurwdt_interrupt (int irq,void *dev_id,struct pt_regs *regs) +static irqreturn_t eurwdt_interrupt (int intr,void *dev_id,struct pt_regs *regs) {printk (KERN_CRIT "timeout WDT timeout\n" );
drivers/char/watchdog/wdt.c +1 -1
@@ -232,7 +232,7 @@ static int wdt_get_temperature(int *temp* a failure condition occurring. */ -static irqreturn_t wdt_interrupt (int irq,void *dev_id,struct pt_regs *regs) +static irqreturn_t wdt_interrupt (int intr,void *dev_id,struct pt_regs *regs) {/* * Read the status register see what is up and
drivers/char/watchdog/wdt_pci.c +1 -1
@@ -277,7 +277,7 @@ static int wdtpci_get_temperature(int *t* a failure condition occurring. */ -static irqreturn_t wdtpci_interrupt (int irq,void *dev_id,struct pt_regs *regs) +static irqreturn_t wdtpci_interrupt (int intr,void *dev_id,struct pt_regs *regs) {/* * Read the status register see what is up and
drivers/isdn/pcbit/module.c +1 -1
@@ -33,7 +33,7 @@ static int num_boards;struct pcbit_dev * dev_pcbit[MAX_PCBIT_CARDS ];extern void pcbit_terminate (int board); -extern int pcbit_init_dev (int board,int mem_base,int irq); +extern int pcbit_init_dev (int board,int mem_base,int intr);static int __init pcbit_init (void ) {
drivers/isdn/pcbit/drv.c +5 -5
@@ -70,7 +70,7 @@ static int pcbit_check_msn(struct pcbit_extern void pcbit_deliver (void * data); -int pcbit_init_dev (int board,int mem_base,int irq) +int pcbit_init_dev (int board,int mem_base,int intr) {struct pcbit_dev *dev;isdn_if *dev_if;
@@ -135,7 +135,7 @@ int pcbit_init_dev(int board, int mem_ba* interrupts */ -if (request_irq (irq, &pcbit_irq_handler,0 , pcbit_devname[board], dev) !=0 ) +if (request_irq (intr, &pcbit_irq_handler,0 , pcbit_devname[board], dev) !=0 ) {kfree (dev->b1);kfree (dev->b2);
@@ -146,7 +146,7 @@ int pcbit_init_dev(int board, int mem_bareturn -EIO ; } - dev->irq = irq; + dev->irq = intr;/* next frame to be received */ dev->rcv_seq =0 ;
@@ -158,7 +158,7 @@ int pcbit_init_dev(int board, int mem_ba dev_if =kmalloc (sizeof (isdn_if),GFP_KERNEL );if (!dev_if) { -free_irq (irq, dev); +free_irq (intr, dev);kfree (dev->b1);kfree (dev->b2);iounmap (dev->sh_mem);
@@ -190,7 +190,7 @@ int pcbit_init_dev(int board, int mem_bastrcpy (dev_if->id, pcbit_devname[board]);if (!register_isdn (dev_if)) { -free_irq (irq, dev); +free_irq (intr, dev);kfree (dev->b1);kfree (dev->b2);iounmap (dev->sh_mem);
drivers/mmc/wbsd.c +16 -16
@@ -1255,7 +1255,7 @@ end:* Interrupt handling */ -static irqreturn_t wbsd_irq (int irq,void *dev_id,struct pt_regs *regs) +static irqreturn_t wbsd_irq (int intr,void *dev_id,struct pt_regs *regs) {struct wbsd_host *host = dev_id;int isr;
@@ -1545,7 +1545,7 @@ static void __devexit wbsd_release_dma(s* Allocate/free IRQ. */ -static int __devinit wbsd_request_irq (struct wbsd_host *host,int irq) +static int __devinit wbsd_request_irq (struct wbsd_host *host,int intr) {int ret;
@@ -1553,11 +1553,11 @@ static int __devinit wbsd_request_irq(st* Allocate interrupt. */ - ret =request_irq (irq, wbsd_irq,IRQF_SHARED ,DRIVER_NAME , host); + ret =request_irq (intr, wbsd_irq,IRQF_SHARED ,DRIVER_NAME , host);if (ret)return ret; - host->irq = irq; + host->irq = intr;/* * Set up tasklets.
@@ -1600,7 +1600,7 @@ static void __devexit wbsd_release_irq(s*/ static int __devinit wbsd_request_resources (struct wbsd_host *host, -int base,int irq,int dma) +int base,int intr,int dma) {int ret;
@@ -1614,7 +1614,7 @@ static int __devinit wbsd_request_resour/* * Allocate interrupt. */ - ret =wbsd_request_irq (host, irq); + ret =wbsd_request_irq (host, intr);if (ret)return ret;
@@ -1687,7 +1687,7 @@ static void wbsd_chip_config(struct wbsdstatic int wbsd_chip_validate (struct wbsd_host *host) { -int base, irq, dma; +int base, intr, dma;wbsd_unlock_config (host);
@@ -1702,7 +1702,7 @@ static int wbsd_chip_validate(struct wbs base =wbsd_read_config (host,WBSD_CONF_PORT_HI ) <<8 ; base |=wbsd_read_config (host,WBSD_CONF_PORT_LO ); - irq =wbsd_read_config (host,WBSD_CONF_IRQ ); + intr =wbsd_read_config (host,WBSD_CONF_IRQ ); dma =wbsd_read_config (host,WBSD_CONF_DRQ );
@@ -1713,7 +1713,7 @@ static int wbsd_chip_validate(struct wbs*/ if (base != host->base)return 0 ; -if (irq != host->irq) +if (intr != host->irq)return 0 ;if ((dma != host->dma) && (host->dma !=-1 ))return 0 ;
@@ -1741,8 +1741,8 @@ static void wbsd_chip_poweroff(struct wb* * \*****************************************************************************/ -static int __devinit wbsd_init (struct device *dev,int base,int irq,int dma, -int pnp) +static int __devinit wbsd_init (struct device *dev,int base,int intr, +int dma,int pnp) {struct wbsd_host *host =NULL ;struct mmc_host *mmc =NULL ;
@@ -1773,7 +1773,7 @@ static int __devinit wbsd_init(struct de/* * Request resources. */ - ret =wbsd_request_resources (host, io, irq, dma); + ret =wbsd_request_resources (host, io, intr, dma);if (ret) {wbsd_release_resources (host);wbsd_free_mmc (dev);
@@ -1880,21 +1880,21 @@ static int __devexit wbsd_remove(structstatic int __devinit wbsd_pnp_probe (struct pnp_dev *pnpdev,const struct pnp_device_id *dev_id) { -int io, irq, dma; +int io, intr, dma;/* * Get resources from PnP layer. */ io =pnp_port_start (pnpdev,0 ); - irq =pnp_irq (pnpdev,0 ); + intr =pnp_irq (pnpdev,0 );if (pnp_dma_valid (pnpdev,0 )) dma =pnp_dma (pnpdev,0 );else dma =-1 ; -DBGF ("PnP resources: port %3x irq %d dma %d\n" , io, irq, dma); +DBGF ("PnP resources: port %3x irq %d dma %d\n" , io, intr, dma); -return wbsd_init (&pnpdev->dev, io, irq, dma,1 ); +return wbsd_init (&pnpdev->dev, io, intr, dma,1 ); } static void __devexit wbsd_pnp_remove(struct pnp_dev *dev)
drivers/net/seeq8005.c +1 -1
@@ -437,7 +437,7 @@ inline void wait_for_buffer(struct net_d/* The typical workload of the driver: Handle the network interface interrupts. */ -static irqreturn_t seeq8005_interrupt (int irq,void *dev_id,struct pt_regs * regs) +static irqreturn_t seeq8005_interrupt (int intr,void *dev_id,struct pt_regs *regs) {struct net_device *dev = dev_id;struct net_local *lp;
include/linux/interrupt.h +14 -14
@@ -100,9 +100,9 @@ extern void free_irq(unsigned int, void#endif #ifdef CONFIG_GENERIC_HARDIRQS -extern void disable_irq_nosync (unsigned int irq); -extern void disable_irq (unsigned int irq); -extern void enable_irq (unsigned int irq); +extern void disable_irq_nosync (unsigned int i); +extern void disable_irq (unsigned int i); +extern void enable_irq (unsigned int i);/* * Special lockdep variants of irq disabling/enabling.
@@ -115,41 +115,41 @@ extern void enable_irq(unsigned int irq)* On !CONFIG_LOCKDEP they are equivalent to the normal * irq disable/enable methods. */ -static inline void disable_irq_nosync_lockdep (unsigned int irq) +static inline void disable_irq_nosync_lockdep (unsigned int i) { -disable_irq_nosync (irq); +disable_irq_nosync (i);#ifdef CONFIG_LOCKDEP local_irq_disable ();#endif } -static inline void disable_irq_lockdep (unsigned int irq) +static inline void disable_irq_lockdep (unsigned int i) { -disable_irq (irq); +disable_irq (i);#ifdef CONFIG_LOCKDEP local_irq_disable ();#endif } -static inline void enable_irq_lockdep (unsigned int irq) +static inline void enable_irq_lockdep (unsigned int i) {#ifdef CONFIG_LOCKDEP local_irq_enable ();#endif -enable_irq (irq); +enable_irq (i); }/* IRQ wakeup (PM) control: */ -extern int set_irq_wake (unsigned int irq,unsigned int on); +extern int set_irq_wake (unsigned int i,unsigned int on); -static inline int enable_irq_wake (unsigned int irq) +static inline int enable_irq_wake (unsigned int i) { -return set_irq_wake (irq,1 ); +return set_irq_wake (i,1 ); } -static inline int disable_irq_wake (unsigned int irq) +static inline int disable_irq_wake (unsigned int i) { -return set_irq_wake (irq,0 ); +return set_irq_wake (i,0 ); }#else /* !CONFIG_GENERIC_HARDIRQS */
include/linux/irq.h +64 -64
291 lines not on this page · raw message
include/linux/random.h +2 -2
@@ -42,11 +42,11 @@ struct rand_pool_info {#ifdef __KERNEL__ -extern void rand_initialize_irq (int irq); +extern void rand_initialize_irq (int intr);extern void add_input_randomness (unsigned int type,unsigned int code,unsigned int value); -extern void add_interrupt_randomness (int irq); +extern void add_interrupt_randomness (int intr);extern void get_random_bytes (void *buf,int nbytes);void generate_random_uuid (unsigned char uuid_out[16 ]);
sound/isa/gus/interwave.c +2 -1
@@ -299,7 +299,8 @@ static int __devinit snd_interwave_detecreturn -ENODEV ; } -static irqreturn_t snd_interwave_interrupt (int irq,void *dev_id,struct pt_regs *regs) +static irqreturn_t snd_interwave_interrupt (int intr,void *dev_id, +struct pt_regs *regs) {struct snd_interwave *iwcard = (struct snd_interwave *) dev_id;int loop, max =5 ;
Hi,
Quoted 2 lines · Jesper Juhl
This is a series of patches that try to be an initial step towards making the kernel build -Wshadow clean.
I'm not sure such patches improve situation.
Quoted 1 line
It'll help us keep our namespaces separate.
Nope, it's exactly opposite - now we have separate namespaces and -Wshadow reduces that separation.
Currently you don't have to worry about the universe when you write a piece of code, and more importantly the universe doesn't have to worry about each function and each private variable. I'm not sure changing that is a good idea.
Signature · 1 line
Krzysztof Halasa
Quoted 2 lines · Krzysztof Halasa
Hi,
Hi Krzysztof,
Quoted 5 lines
Quoted 2 lines · Jesper Juhl
This is a series of patches that try to be an initial step towards making the kernel build -Wshadow clean.
Quoted 7 lines
I'm not sure such patches improve situation.
Quoted 1 line
It'll help us keep our namespaces separate.
Nope, it's exactly opposite - now we have separate namespaces and -Wshadow reduces that separation.
I don't agree. -Wshadow lets the compiler help you ensure that you don't accidentally use a symbol from a local scope when you think you are using one from an enclosing scope (global or not). Bugs resulting from such use can be hard to track down and if we can get the compiler to help us avoid them I think that's a win.
Quoted 4 lines
Currently you don't have to worry about the universe when you write a piece of code, and more importantly the universe doesn't have to worry about each function and each private variable. I'm not sure changing that is a good idea.
I think it's a good thing that we have to take a little more care when choosing global function and variable names... Take up() for example - in my (very humble) oppinion that is a very bad name for a global function - it clashes too easily with local function and variable names, and a programmer who's not careful may end up calling the global up() when he wants the local and vice versa (a much better name would have been sem_up() - should we change that???).
I think it's a good think if we in the future name our global stuff with more care and stick a big fat warning in the face of programmers who introduce local stuff that shadows something else.
I don't agree with you and I don't know how to convince you, but I still appreciate your feedback.
Thanks.
I'll leave it to people higher in the hierarchy to decide if these patches should be applied or not ;)
Keep that feedback flowing people :-)
Signature · 3 lines
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
Quoted 7 lines · Jesper Juhl
I think it's a good thing that we have to take a little more care when choosing global function and variable names... Take
up()for example - in my (very humble) oppinion that is a very bad name for a global function - it clashes too easily with local function and variable names, and a programmer who's not careful may end up calling the globalup()when he wants the local and vice versa (a much better name would have beensem_up()- should we change that???).
Possibly, but it could then conflict with something else. Anytime we add/change some global symbol, we would have to scan entire kernel for conflicts (authors of (yet) off-tree things would hate us). I don't think it's practical, especially with, IMHO, no real gain.
Quoted 3 lines
I don't agree with you and I don't know how to convince you, but I still appreciate your feedback.
Thanks.
You're welcome. I'd be more happy if I could say I like the idea :-(
Quoted 2 lines
I'll leave it to people higher in the hierarchy to decide if these patches should be applied or not ;)
Sure.
Signature · 1 line
Krzysztof Halasa
Quoted 10 lines · On Sun, 30 Jul 2006 19:27:36 +0200 Krzysztof Halasa <[email protected]> wrote:
Quoted 7 lines · Jesper Juhl
I think it's a good thing that we have to take a little more care when choosing global function and variable names... Take
up()for example - in my (very humble) oppinion that is a very bad name for a global function - it clashes too easily with local function and variable names, and a programmer who's not careful may end up calling the globalup()when he wants the local and vice versa (a much better name would have beensem_up()- should we change that???).
Quoted 3 lines
Possibly, but it could then conflict with something else. Anytime we add/change some global symbol, we would have to scan entire kernel for conflicts (authors of (yet) off-tree things would hate us).
These things happen. And it's only a warning.
Quoted 1 line
I don't think it's practical, especially with, IMHO, no real gain.
While I don't recall any kernel bugs which -Wshadow would have saved us from, I think it's a sensible thing to do - it _might_ save us from a bug, and we need all the help we can get.
Plus it's often the case that if a local and a global clash, one of the identifiers was poorly chosen.
Arnd Bergmann
maintainerAm Sunday 30 July 2006 20:29 schrieb Andrew Morton:
Quoted 3 lines
While I don't recall any kernel bugs which -Wshadow would have saved us from, I think it's a sensible thing to do - it _might_ save us from a bug, and we need all the help we can get.
One case where it would have helped in the past is jiffies -- when experimenting with tickless systems, turning the global jiffies variable into a macro comes in handy, but that breaks all functions that have a local variable with the same name.
Arnd <><
Quoted 9 lines
Quoted 7 lines
I think it's a good thing that we have to take a little more care when choosing global function and variable names... Take
up()for example - in my (very humble) oppinion that is a very bad name for a global function - it clashes too easily with local function and variable names, and a programmer who's not careful may end up calling the globalup()when he wants the local and vice versa (a much better name would have beensem_up()- should we change that???).(authors of (yet) off-tree things would hate us)
Mark up() as deprecated while sem_up() emerges - hey, we even have an __attribute__(()) for that..
Jan Engelhardt
Quoted 5 lines · Jesper Juhl
Ok, here we go again...
This is a series of patches that try to be an initial step towards making the kernel build -Wshadow clean.
Replying to myself here since I forgot one little bit.
It would be great if maintainers of the various areas that my patches touch would explicitly ack or nack patches - preferably giving reasons for nack's as well.
That would help me a lot in updating the patch-set (if so needed).
Thanks.
Signature · 3 lines
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
Quoted 9 lines · Jesper Juhl
Quoted 5 lines · Jesper Juhl
Ok, here we go again...
This is a series of patches that try to be an initial step towards making the kernel build -Wshadow clean.
It would be great if maintainers of the various areas that my patches touch would explicitly ack or nack patches - preferably giving reasons for nack's as well.
My move to a new town is basically done, so I'll give it a run for the VAX specific bits which I care about.
MfG, JBG
Signature · 3 lines
Jan-Benedict Glaw [email protected] +49-172-7608481 Signature of: If it doesn't work, force it. the second : If it breaks, it needed replacing anyway.
Quoted 4 lines · Jesper Juhl
Ok, here we go again...
This is a series of patches that try to be an initial step towards making the kernel build -Wshadow clean.
I will take care of warnings in scripts/*
mconf/lxdialog warnings will be fixed in my lxdialog tree which has enough patches to make your path of no real use.
And its a trivial fix from my side.
Sam
Quoted 10 lines · Sam Ravnborg
Quoted 4 lines · Jesper Juhl
Ok, here we go again...
This is a series of patches that try to be an initial step towards making the kernel build -Wshadow clean.
I will take care of warnings in
scripts/*
mconf/lxdialog warnings will be fixed in my lxdialog tree which has enough patches to make your path of no real use.
And its a trivial fix from my side.
Great. I'll drop everything in scripts/ and rely on you there. Thanks a lot for the feedback.
Signature · 3 lines
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