Patchwork [02/27] drivers/net: fix sparse warnings: make do-while a compound statement

login
register
mail settings
Submitter Hannes Eder
Date Dec. 22, 2008, 7:15 p.m.
Message ID <20081222191507.11807.50794.stgit@vmbox.hanneseder.net>
Download mbox | patch
Permalink /patch/15270/
State Accepted
Delegated to: David Miller
Headers show

Comments

Hannes Eder - Dec. 22, 2008, 7:15 p.m.
While at it insert some extra curly braces and fix formatting.

Fix this sparse warnings:

  drivers/net/atp.c:811:8: warning: do-while statement is not a compound statement
  drivers/net/atp.c:813:8: warning: do-while statement is not a compound statement
  drivers/net/atp.c:815:11: warning: do-while statement is not a compound statement
  drivers/net/atp.c:817:11: warning: do-while statement is not a compound statement
  drivers/net/plip.c:642:4: warning: do-while statement is not a compound statement
  drivers/net/plip.c:647:4: warning: do-while statement is not a compound statement
  drivers/net/plip.c:820:4: warning: do-while statement is not a compound statement
  drivers/net/plip.c:825:4: warning: do-while statement is not a compound statement
  drivers/net/starfire.c:886:3: warning: do-while statement is not a compound statement

Signed-off-by: Hannes Eder <hannes@hanneseder.net>
---
 drivers/net/atp.c      |   19 ++++++++++---------
 drivers/net/plip.c     |   16 ++++++++--------
 drivers/net/starfire.c |    4 ++--
 3 files changed, 20 insertions(+), 19 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Halasa - Dec. 22, 2008, 10:14 p.m.
Hannes Eder <hannes@hanneseder.net> writes:

> Fix this sparse warnings:

Or: break the formating to silence sparse.

>   drivers/net/atp.c:811:8: warning: do-while statement is not a compound statement
>   drivers/net/atp.c:813:8: warning: do-while statement is not a compound statement
>   drivers/net/atp.c:815:11: warning: do-while statement is not a compound statement
>   drivers/net/atp.c:817:11: warning: do-while statement is not a compound statement

> --- a/drivers/net/starfire.c
> +++ b/drivers/net/starfire.c
> @@ -882,9 +882,9 @@ static int mdio_read(struct net_device *dev, int phy_id, int location)
>  	void __iomem *mdio_addr = np->base + MIICtrl + (phy_id<<7) + (location<<2);
>  	int result, boguscnt=1000;
>  	/* ??? Should we add a busy-wait here? */
> -	do
> +	do {
>  		result = readl(mdio_addr);
> -	while ((result & 0xC0000000) != 0x80000000 && --boguscnt > 0);
> +	} while ((result & 0xC0000000) != 0x80000000 && --boguscnt > 0);
>  	if (boguscnt == 0)
>  		return 0;
>  	if ((result & 0xffff) == 0xffff)


I don't know how one could think the above is an improvement.

This "do-while statement is not a compound statement" warning is
pure nonsense.
=?UTF-8?Q?H=C3=A5kon_L=C3=B8vdal?= - Dec. 22, 2008, 11:44 p.m.
2008/12/22 Krzysztof Halasa <khc@pm.waw.pl>:
>> --- a/drivers/net/starfire.c
>> +++ b/drivers/net/starfire.c
>> @@ -882,9 +882,9 @@ static int mdio_read(struct net_device *dev, int phy_id, int location)
>>       void __iomem *mdio_addr = np->base + MIICtrl + (phy_id<<7) + (location<<2);
>>       int result, boguscnt=1000;
>>       /* ??? Should we add a busy-wait here? */
>> -     do
>> +     do {
>>               result = readl(mdio_addr);
>> -     while ((result & 0xC0000000) != 0x80000000 && --boguscnt > 0);
>> +     } while ((result & 0xC0000000) != 0x80000000 && --boguscnt > 0);
>>       if (boguscnt == 0)
>>               return 0;
>>       if ((result & 0xffff) == 0xffff)
>
>
> I don't know how one could think the above is an improvement.

For this specific issue the important aspect is to improve readability
(and not just eventually satisfy some warning from a tool), which I
assume there is no disagrement on. Now what constitutes improved
readbility on the other hand is another issue, and I guess there are
next to as many oppinions as developers... :)
I would most certainly prefer to have code look consistently and always
have brackets, even in the case of just one body line.

Of secondary importance is the benefit that always using brackets
makes them much more merge friendly. Consider the following scenario:

Common base:
        do
                result = readl(mdio_addr);
        while ((result & 0xC0000000) != 0x80000000 && --boguscnt > 0);

Branch 1
        do {
                result = readl(mdio_addr);
                do_something();
        } while ((result & 0xC0000000) != 0x80000000 && --boguscnt > 0);

Branch 2
        do
                result = readl(mdio_addr);
        while (NOT_OK(result) && --boguscnt > 0);

In this case if both branch 1 and 2 are merged, there will be a merge conflict
that has to be resolved manually. If the brackets had been in place from the
start tools should be able to resolv this automatically.

BR Håkon Løvdal
Krzysztof Halasa - Dec. 23, 2008, 4:31 p.m.
"Håkon Løvdal" <hlovdal@gmail.com> writes:

> For this specific issue the important aspect is to improve readability
> (and not just eventually satisfy some warning from a tool), which I
> assume there is no disagrement on.

Precisely.

> Now what constitutes improved
> readbility on the other hand is another issue, and I guess there are
> next to as many oppinions as developers... :)

Yes. Thus the brackets (or lack of them) of do-while loops should
never be forced as a part of coding standard/tools. This is just an
unimportant detail of personal style and trying to force it is simply
damaging to all of us.

> Of secondary importance is the benefit that always using brackets
> makes them much more merge friendly.

There are many ways to make the code more merge friendly at a cost of
readability. Hope we don't go this way.
Harvey Harrison - Dec. 23, 2008, 5:26 p.m.
On Tue, 2008-12-23 at 17:31 +0100, Krzysztof Halasa wrote:
> "Håkon Løvdal" <hlovdal@gmail.com> writes:
> 
> > For this specific issue the important aspect is to improve readability
> > (and not just eventually satisfy some warning from a tool), which I
> > assume there is no disagrement on.
> 
> Precisely.
> 
> > Now what constitutes improved
> > readbility on the other hand is another issue, and I guess there are
> > next to as many oppinions as developers... :)
> 
> Yes. Thus the brackets (or lack of them) of do-while loops should
> never be forced as a part of coding standard/tools. This is just an
> unimportant detail of personal style and trying to force it is simply
> damaging to all of us.
> 
> > Of secondary importance is the benefit that always using brackets
> > makes them much more merge friendly.
> 
> There are many ways to make the code more merge friendly at a cost of
> readability. Hope we don't go this way.

Linus himself added that particular warning to sparse...may want to check
with him the reason for it.

Harvey

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Halasa - Dec. 23, 2008, 5:36 p.m.
Harvey Harrison <harvey.harrison@gmail.com> writes:

>> There are many ways to make the code more merge friendly at a cost of
>> readability. Hope we don't go this way.
>
> Linus himself added that particular warning to sparse...may want to check
> with him the reason for it.

Once again, this is a personal thing, and a harmless one.

Sometimes I think using this sparse thing does more harm than good.
Not that the tool itself is bad - I find it really useful, many sparse
warnings actually did cause code improvement.

It's like with knives, there are good uses and a bit less good ones.
Linus Torvalds - Dec. 23, 2008, 6:08 p.m.
On Tue, 23 Dec 2008, Krzysztof Halasa wrote:

> Harvey Harrison <harvey.harrison@gmail.com> writes:
> 
> >> There are many ways to make the code more merge friendly at a cost of
> >> readability. Hope we don't go this way.
> >
> > Linus himself added that particular warning to sparse...may want to check
> > with him the reason for it.
> 
> Once again, this is a personal thing, and a harmless one.

It's more than that. I added the check after some person who had been 
programming the kernel (and thus was supposedly fluent in C) literally 
could not parse a macro that had "do x while (y)" in it.

Why? Because it's so uncommon, and because "while (y)" on its own means 
something totally different.

So the syntactic sugar to _always_ have do-while loops have that brace is 
a way to avoid one of the rather few places where the C language has 
syntax that is very context-dependent.

Another example of this is "sizeof". The kernel universally (I hope) has 
parenthesis around the sizeof argument, even though it's clearly not 
required by the C language. 

It's a coding standard. 

And quite frankly, anybody who works on gcc has no place complaining about 
sparse coding standard warnings. They are a _hell_ of a lot better than 
some of the really crazy warnings gcc spews out with "-W". At least the 
sparse warnings you can make go away while making the code more 
understandable. Some of the -W warnings are unfixable without breaking the 
source code.

		Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Halasa - Dec. 23, 2008, 11:18 p.m.
Linus Torvalds <torvalds@linux-foundation.org> writes:

> It's more than that. I added the check after some person who had been 
> programming the kernel (and thus was supposedly fluent in C) literally 
> could not parse a macro that had "do x while (y)" in it.

A literally that simple macro, or a complicated one?


I'm all for using brackets when there is / could be some possibility
of increasing code readability.

E.g. I always use parentheses in a nested if-if, because

	if (x)
		if (y)
			a;
		else
			c;

may be confusing, especially if formatted differently.

	if (x)
		a;
	else if (y)
		b;
	etc.

is simple and unambiguous and I don't put the braces.

So is a case like
	do
		x;
	while (y);
It can't be made more clear with brackets.

IOW: improving the style is great. Changing it only to silence some
tool is not.

> Another example of this is "sizeof". The kernel universally (I hope) has 
> parenthesis around the sizeof argument, even though it's clearly not 
> required by the C language. 
>
> It's a coding standard.

Right, but they (at least for me) make it more readable.
kmalloc(sizeof i) just doesn't look good, the operator looks like
a variable name.

But there is this return statement. Some people tend to write
return (x); I simply write return x;
It's clear, and so is a simple do-while.

> And quite frankly, anybody who works on gcc has no place complaining about 
> sparse coding standard warnings. They are a _hell_ of a lot better than 
> some of the really crazy warnings gcc spews out with "-W". At least the 
> sparse warnings you can make go away while making the code more 
> understandable. Some of the -W warnings are unfixable without breaking the 
> source code.

:-)

BTW I think I may use sparse differently.
I can see false gcc warnings every time the project is being built.
OTOH I run sparse only when I have some (almost) completed project
(a patch, a driver etc). I make sure the remaining sparse warnings are
(from my POV) invalid and it won't spew them on next build again.
Linus Torvalds - Dec. 23, 2008, 11:38 p.m.
On Wed, 24 Dec 2008, Krzysztof Halasa wrote:
> 
> So is a case like
> 	do
> 		x;
> 	while (y);
> It can't be made more clear with brackets.

Oh yes it can. People look at that, and it's so uncommon that they 
literally believe it is a mis-indent.

Your example with nested if-statements are totally pointless, because you 
didn't even apparently understand my comment about "while()" having two 
totally different meanings (which is not true of "if()"), nor realize the 
importance of how common something is.

Common patterns become things that people take for granted and don't have 
any trouble with. In contrast, do-while without braces is _extremely_ 
uncommon.

> IOW: improving the style is great. Changing it only to silence some
> tool is not.

Sorry, you're wrong. It's not changed to silence some tool. THIS IS THE 
KERNEL CODING STYLE. 

I don't care one whit about your personal coding style. The kernel has 
brances. End of discussion. sparse complains about lack of them. 
Comprende?

> Right, but they (at least for me) make it more readable.

I don't care.

> kmalloc(sizeof i) just doesn't look good, the operator looks like
> a variable name.

And I agree with you. "sizeof i" doesn't look good. It's uncommon, and 
doesn't match peoples expectations.

> But there is this return statement. Some people tend to write
> return (x); I simply write return x;

I do to. And it's the common thing to do, and only totally confused people 
think that 'return' is a somehow remotely like a "function" of its 
arguments (the way 'sizeof' is - sizeof _is_ a function of its arguments, 
albeit a very rare one).

> It's clear, and so is a simple do-while.

No. "return x;" is clear because it's the common thing, which means that 
peopel are good at reading it.

Another example of "common vs non-common" is this:

	if (0 <= x)
		do something..

is something that crazy people do (sadly, one of the crazy people taught 
the git maintainer C programming, so now even sane people do it). It's 
crazy because it's uncommon, which means that most people have to think 
about it A LOT MORE than about

	if (x >= 0)
		do something..

even though technically both are obviously EXACTLY THE SAME THING.

Can you see the argument? Doing things the common way is important, 
because it allows people to see what they mean without having to think 
about it. They just scan it, and the meaning is clear.

And that's why "do while" without braces is bad. If you scan it quickly on 
its own, you may well end up just seeing the

	while (x);

part, and get confused ("oh, a delay loop"). But if you see

	} while (x);

you aren't confused, because the latter one is clearly an ending condition 
of a do-while loop, IN A WAY THAT THE FIRST ONE IS NOT!

See?

do-while is very special, because as mentioned, "while" is a really magic 
C keyword that has two TOTALLY DIFFERENT meanings. Don't make people look 
for the "do".

		Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Halasa - Dec. 24, 2008, 2:03 a.m.
Linus Torvalds <torvalds@linux-foundation.org> writes:

> Oh yes it can. People look at that, and it's so uncommon that they 
> literally believe it is a mis-indent.

People learn, or should, through the life :-)

I'm not sure being common or less common does matter here much.

OTOH I think it's pretty common. Approx as common as while (x) y is,
isn't it?

Except in macros: do-while(0) is commonly used in macros because it
"eats" the semicolon.

(do { xxx; yyy; zzz } while (0) is also quite common instead of "goto"
to add multiple exits from a long block).

> Your example with nested if-statements are totally pointless, because you 
> didn't even apparently understand my comment about "while()" having two 
> totally different meanings (which is not true of "if()"), nor realize the 
> importance of how common something is.

Only apparently. If there is no ambiguity, there is no problem, the
double meaning does not seem relevant.

You tell me that

	do
		expr;
	while (condition);

(with precisely this formatting) can be unclear?

Or you say

	do {
		expr;
	} while (condition);

is better? If it is, then certainly not for me.


Now: are we going to write {} because you say so, even if it makes the
code worse? Or should we rather aim at improving the code, even if
only a bit? (I admit that, IMO, the second example is worse = less
readable, but only a small bit).

> Sorry, you're wrong. It's not changed to silence some tool.

Come on, you have it plain in the subject - "fix sparse warnings".
If it was "improve obscure notation" I wouldn't say a word.

> And I agree with you. "sizeof i" doesn't look good. It's uncommon, and 
> doesn't match peoples expectations.

But it doesn't matter if it's common or not. It doesn't look good and
THIS IS WHY it's uncommon/nonexistent. Not the other way around.

There are many uncommon things in a project like a Linux kernel. There
are some macros which take me a while :-) to understand and they
thankfully don't spew warnings.

> Another example of "common vs non-common" is this:
>
> 	if (0 <= x)
> 		do something..
>
> is something that crazy people do (sadly, one of the crazy people taught 
> the git maintainer C programming, so now even sane people do it). It's 
> crazy because it's uncommon, which means that most people have to think 
> about it A LOT MORE than about
>
> 	if (x >= 0)
> 		do something..

No. It's crazy not because it's uncommon, but because this is how we
have been taught in school.

I don't know reasons for "0 >= x" but I know one for
	if (0 == x)
		do something..

It's because people sometimes write "=" instead of "==" and "0 = x"
doesn't make sense to gcc. I think it's not a valid reason, and
(unless you use (()) which is IMHO also crazy) gcc will warn about
if (x = 0) (though I think the warning is stupid and "if (a = b)"
is a valid usage if a and b are short).

It's simple: one has to see the difference between if (x = 0) vs
if (x == 0), there is no way around it. if (0 >= x), I don't know.

> Can you see the argument? Doing things the common way is important, 
> because it allows people to see what they mean without having to think 
> about it. They just scan it, and the meaning is clear.

Then I must be different. Things are clear to me if they are clear,
even if they are uncommon. And common things aren't necessarily clear.
Hmm, I should have expected something like that.

> And that's why "do while" without braces is bad. If you scan it quickly on 
> its own, you may well end up just seeing the
>
> 	while (x);
>
> part, and get confused ("oh, a delay loop").

Nope.
It raises a red flag in my head immediately. We don't do such busy
loops generally, and if we do, I would write it as:

	while (x)
		;

> But if you see
>
> 	} while (x);
>
> you aren't confused, because the latter one is clearly an ending condition 
> of a do-while loop, IN A WAY THAT THE FIRST ONE IS NOT!
>
> See?

No. I must be different.

> do-while is very special, because as mentioned, "while" is a really magic 
> C keyword that has two TOTALLY DIFFERENT meanings.

Probably. I just don't see it.

For me, every code fragment is different, and a program like sparse
can't even come close to know what's clear for me and what is not.

I use sparse as a great help - but only that, a help.
Linus Torvalds - Dec. 24, 2008, 2:10 a.m.
On Wed, 24 Dec 2008, Krzysztof Halasa wrote:
> 
> People learn, or should, through the life :-)

Sure. But you should learn about the things that matter - not learn to 
avoid the stupid pitfalls that come from confusingly doing things so that 
they visually look similar even when they do different things.

So don't make people learn by putting traps in their face. That just 
wastes everybodys time.

> I'm not sure being common or less common does matter here much.
> 
> OTOH I think it's pretty common. Approx as common as while (x) y is,
> isn't it?

I doubt it. It certainly wasn't in the kernel. When we added the sparse 
warning, I think we got a couple of hits. 

Anyway,  not worth discussing. The fact is, the kernel does not accept do 
while without braces. I told you why. You can ignore it. I'll ignore you.

		Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Junio C Hamano - Dec. 25, 2008, 6:17 a.m.
Krzysztof Halasa <khc@pm.waw.pl> writes:

> Linus Torvalds <torvalds@linux-foundation.org> writes:
> ...
>> Another example of "common vs non-common" is this:
>>
>> 	if (0 <= x)
>> 		do something..
>>
>> is something that crazy people do (sadly, one of the crazy people taught 
>> the git maintainer C programming, so now even sane people do it). It's 
>> crazy because it's uncommon, which means that most people have to think 
>> about it A LOT MORE than about
>>
>> 	if (x >= 0)
>> 		do something..
>
> No. It's crazy not because it's uncommon, but because this is how we
> have been taught in school.
>
> I don't know reasons for "0 >= x" but I know one for
> 	if (0 == x)
> 		do something..
>
> It's because people sometimes write "=" instead of "==" and "0 = x"
> doesn't make sense to gcc.

It does not have anything to do with the assignment confusion.

It is "textual order should reflect actual order" (aka "have number line
in your head when you write your comparison conditional"):

    http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126

Even if it may make logical sense, I would not suggest using it when other
people are not familiar with the convention, though.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Halasa - Dec. 25, 2008, 5:02 p.m.
Linus Torvalds <torvalds@linux-foundation.org> writes:

> Anyway,  not worth discussing. The fact is, the kernel does not accept do 
> while without braces. I told you why. You can ignore it. I'll ignore you.

Oh, I'm not a martyr.
David Miller - Dec. 26, 2008, 7:55 a.m.
From: Hannes Eder <hannes@hanneseder.net>
Date: Mon, 22 Dec 2008 20:15:07 +0100

> While at it insert some extra curly braces and fix formatting.
...
> Signed-off-by: Hannes Eder <hannes@hanneseder.net>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hannes Eder - Dec. 29, 2008, 2:35 p.m.
On Wed, Dec 24, 2008 at 12:38 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
>
> On Wed, 24 Dec 2008, Krzysztof Halasa wrote:
>>
>> So is a case like
>>       do
>>               x;
>>       while (y);
>> It can't be made more clear with brackets.
>
> Oh yes it can. People look at that, and it's so uncommon that they
> literally believe it is a mis-indent.
>
> [snip]
>
>> kmalloc(sizeof i) just doesn't look good, the operator looks like
>> a variable name.
>
> And I agree with you. "sizeof i" doesn't look good. It's uncommon, and
> doesn't match peoples expectations.
>
>> But there is this return statement. Some people tend to write
>> return (x); I simply write return x;
>
> I do to. And it's the common thing to do, and only totally confused people
> think that 'return' is a somehow remotely like a "function" of its
> arguments (the way 'sizeof' is - sizeof _is_ a function of its arguments,
> albeit a very rare one).
>
>> It's clear, and so is a simple do-while.
>
> No. "return x;" is clear because it's the common thing, which means that
> peopel are good at reading it.

I got curious and conducted a little experiment, here is the outcome
for the linux-2.6 kernel tree:

hannes@vmbox:~/linux-2.6$ find . -name "*.[ch]" -print0 | xargs -0 cat
| ../sparse/cstats -

stats for '-':
  do's:         8092, non compound:              79 (  1.0%)
  sizeof's:    51216, without parenthesis:     1543 (  3.0%)
  return's:   286167, with parenthesis:       13552 (  4.7%)

'cstats' is a little program I wrote using the sparse library, see below.

The value for "return's with parenthesis" is a bit of an estimation,
as 'cstats' operates only at the token level, so
"return (x) ? y : z;" counts as "return with parenthesis".

some sanity checks, to ensure the magnitudes are right:

hannes@vmbox:~/linux-2.6$ git grep -w do -- *.[ch] | wc -l
20599

hannes@vmbox:~/linux-2.6$ git grep '\bdo[ \t]*{' -- *.[ch] | wc -l
7805

I assume 'do' is used frequently in comments:

hannes@vmbox:~/linux-2.6$ git grep '\*.*\bdo\b' -- *.[ch] | wc -l
11358

hannes@vmbox:~/linux-2.6$ git grep '^[ \t]*do$' -- *.[ch] | wc -l
34

hannes@vmbox:~/linux-2.6$ git grep -w sizeof -- *.[ch] | wc -l
49631

constructs like sizeof(array)/sizeof(array[0]) or common:

hannes@vmbox:~/linux-2.6$ git grep  'sizeof.*sizeof' -- *.[ch] | wc -l
1827

hannes@vmbox:~/linux-2.6$ git grep '\bsizeof [^(]' -- *.[ch] | wc -l
1534

hannes@vmbox:~/linux-2.6$ git grep -w return -- *.[ch] | wc -l
295304

hannes@vmbox:~/linux-2.6$ git grep '\breturn (' -- *.[ch] | wc -l
11067

Best,
Hannes


#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>

#include "token.h"
#include "parse.h"

int main(int argc, char **argv)
{
        struct string_list *filelist = NULL;
        char *filename;

        sparse_initialize(argc, argv, &filelist);
        FOR_EACH_PTR_NOTAG(filelist, filename) {
                int fd;
                struct token *token;
                int do_stats = 0;
                int do_stats_nc = 0;
                int sizeof_exprs = 0;
                int sizeof_exprs_np = 0;
                int return_stats = 0;
                int return_stats_wp = 0;

                if (strcmp(filename, "-") == 0) {
                        fd = 0;
                } else {
                        fd = open(filename, O_RDONLY);
                        if (fd < 0)
                                die("No such file: %s", filename);
                }

                // Tokenize the input stream
                token = tokenize(filename, fd, NULL, includepath);
                close(fd);

                for ( ; !eof_token(token); token = token->next) {
                        if (token_type(token) == TOKEN_IDENT) {
                                if (token->ident == &do_ident) {
                                        do_stats++;
                                        if (!match_op(token->next, '{'))
                                                do_stats_nc++;
                                }
                                else if (token->ident == &sizeof_ident) {
                                        sizeof_exprs++;
                                        if (!match_op(token->next, '('))
                                                sizeof_exprs_np++;
                                }
                                else if (token->ident == &return_ident) {
                                        return_stats++;
                                        if (match_op(token->next, '('))
                                                return_stats_wp++;
                                }
                        }
                }

                printf("stats for '%s':\n", filename);
                printf("  do's:     %8d, non compound:        %8d (%5.1f%%)\n",
                       do_stats, do_stats_nc,
                       100.0 * do_stats_nc / (do_stats?:1) );
                printf("  sizeof's: %8d, without parenthesis: %8d (%5.1f%%)\n",
                       sizeof_exprs, sizeof_exprs_np,
                       100.0 * sizeof_exprs_np / (sizeof_exprs?:1) );
                printf("  return's: %8d, with parenthesis:    %8d (%5.1f%%)\n",
                       return_stats, return_stats_wp,
                       100.0 * return_stats_wp / (return_stats?:1) );

        } END_FOR_EACH_PTR_NOTAG(filename);
        return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/atp.c b/drivers/net/atp.c
index 1d6b74c..ea493ce 100644
--- a/drivers/net/atp.c
+++ b/drivers/net/atp.c
@@ -802,21 +802,22 @@  static void net_rx(struct net_device *dev)
 
 static void read_block(long ioaddr, int length, unsigned char *p, int data_mode)
 {
-
 	if (data_mode <= 3) { /* Mode 0 or 1 */
 		outb(Ctrl_LNibRead, ioaddr + PAR_CONTROL);
 		outb(length == 8  ?  RdAddr | HNib | MAR  :  RdAddr | MAR,
 			 ioaddr + PAR_DATA);
 		if (data_mode <= 1) { /* Mode 0 or 1 */
-			do  *p++ = read_byte_mode0(ioaddr);  while (--length > 0);
-		} else	/* Mode 2 or 3 */
-			do  *p++ = read_byte_mode2(ioaddr);  while (--length > 0);
-	} else if (data_mode <= 5)
-		do      *p++ = read_byte_mode4(ioaddr);  while (--length > 0);
-	else
-		do      *p++ = read_byte_mode6(ioaddr);  while (--length > 0);
+			do { *p++ = read_byte_mode0(ioaddr); } while (--length > 0);
+		} else { /* Mode 2 or 3 */
+			do { *p++ = read_byte_mode2(ioaddr); } while (--length > 0);
+		}
+	} else if (data_mode <= 5) {
+		do { *p++ = read_byte_mode4(ioaddr); } while (--length > 0);
+	} else {
+		do { *p++ = read_byte_mode6(ioaddr); } while (--length > 0);
+	}
 
-    outb(EOC+HNib+MAR, ioaddr + PAR_DATA);
+	outb(EOC+HNib+MAR, ioaddr + PAR_DATA);
 	outb(Ctrl_SelData, ioaddr + PAR_CONTROL);
 }
 
diff --git a/drivers/net/plip.c b/drivers/net/plip.c
index 5d904f7..ed8582e 100644
--- a/drivers/net/plip.c
+++ b/drivers/net/plip.c
@@ -638,14 +638,14 @@  plip_receive_packet(struct net_device *dev, struct net_local *nl,
 
 	case PLIP_PK_DATA:
 		lbuf = rcv->skb->data;
-		do
+		do {
 			if (plip_receive(nibble_timeout, dev,
 					 &rcv->nibble, &lbuf[rcv->byte]))
 				return TIMEOUT;
-		while (++rcv->byte < rcv->length.h);
-		do
+		} while (++rcv->byte < rcv->length.h);
+		do {
 			rcv->checksum += lbuf[--rcv->byte];
-		while (rcv->byte);
+		} while (rcv->byte);
 		rcv->state = PLIP_PK_CHECKSUM;
 
 	case PLIP_PK_CHECKSUM:
@@ -816,14 +816,14 @@  plip_send_packet(struct net_device *dev, struct net_local *nl,
 		snd->checksum = 0;
 
 	case PLIP_PK_DATA:
-		do
+		do {
 			if (plip_send(nibble_timeout, dev,
 				      &snd->nibble, lbuf[snd->byte]))
 				return TIMEOUT;
-		while (++snd->byte < snd->length.h);
-		do
+		} while (++snd->byte < snd->length.h);
+		do {
 			snd->checksum += lbuf[--snd->byte];
-		while (snd->byte);
+		} while (snd->byte);
 		snd->state = PLIP_PK_CHECKSUM;
 
 	case PLIP_PK_CHECKSUM:
diff --git a/drivers/net/starfire.c b/drivers/net/starfire.c
index 01493a4..8bfc15b 100644
--- a/drivers/net/starfire.c
+++ b/drivers/net/starfire.c
@@ -882,9 +882,9 @@  static int mdio_read(struct net_device *dev, int phy_id, int location)
 	void __iomem *mdio_addr = np->base + MIICtrl + (phy_id<<7) + (location<<2);
 	int result, boguscnt=1000;
 	/* ??? Should we add a busy-wait here? */
-	do
+	do {
 		result = readl(mdio_addr);
-	while ((result & 0xC0000000) != 0x80000000 && --boguscnt > 0);
+	} while ((result & 0xC0000000) != 0x80000000 && --boguscnt > 0);
 	if (boguscnt == 0)
 		return 0;
 	if ((result & 0xffff) == 0xffff)