lkml.org 
[lkml]   [2008]   [Dec]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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@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;
    }


    \
     
     \ /
      Last update: 2008-12-29 15:39    [W:5.032 / U:0.292 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site