| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <154e089b0812290635o5792af25jd26fa184a02dc7af@mail.gmail.com>
Date: Mon, 29 Dec 2008 15:35:31 +0100
From: "Hannes Eder" <hannes@...neseder.net>
To: "Linus Torvalds" <torvalds@...ux-foundation.org>
Cc: "Krzysztof Halasa" <khc@...waw.pl>,
"Harvey Harrison" <harvey.harrison@...il.com>,
"Håkon Løvdal" <hlovdal@...il.com>,
netdev@...r.kernel.org, kernel-janitors@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a compound statement
On Wed, Dec 24, 2008 at 12:38 AM, Linus Torvalds
<torvalds@...ux-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@...ox:~/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@...ox:~/linux-2.6$ git grep -w do -- *.[ch] | wc -l
20599
hannes@...ox:~/linux-2.6$ git grep '\bdo[ \t]*{' -- *.[ch] | wc -l
7805
I assume 'do' is used frequently in comments:
hannes@...ox:~/linux-2.6$ git grep '\*.*\bdo\b' -- *.[ch] | wc -l
11358
hannes@...ox:~/linux-2.6$ git grep '^[ \t]*do$' -- *.[ch] | wc -l
34
hannes@...ox:~/linux-2.6$ git grep -w sizeof -- *.[ch] | wc -l
49631
constructs like sizeof(array)/sizeof(array[0]) or common:
hannes@...ox:~/linux-2.6$ git grep 'sizeof.*sizeof' -- *.[ch] | wc -l
1827
hannes@...ox:~/linux-2.6$ git grep '\bsizeof [^(]' -- *.[ch] | wc -l
1534
hannes@...ox:~/linux-2.6$ git grep -w return -- *.[ch] | wc -l
295304
hannes@...ox:~/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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists